feat: Using SameAs generator for dynamic binding of permissions

[mesemus]

Warning

This is a backward-incompatible change, as users might, in rare cases, rely on the static binding inside the base permission policy.

Description

This PR replaces static binding of permissions with a dynamic one using SameAs.
It depends on inveniosoftware/invenio-records-permissions#119

Rationale

A permission policy can be a complicated thing - look for example at the RDM permission policy:

class RDMPermissionPolicy:
    can_manage = [
        RecordOwners(),
        RecordCommunitiesAction("curate"),
        AccessGrant("manage"),
        SystemProcess(),
    ]
    can_curate = can_manage + [AccessGrant("edit"), SecretLinks("edit")]

If we inherit from the policy and modify the can_manage to include another group of users:

class MyPP(RDMPermissionPolicy):
    can_manage = [
        UsersWithRole("myrole"),
        RecordOwners(),
        RecordCommunitiesAction("curate"),
        AccessGrant("manage"),
        SystemProcess(),
    ]

can_curate is not updated as a user might expect. When evaluating permissions, can_curate will not contain the UsersWithRole generator because it was created statically inside the RDMPermissionPolicy class.

The SameAs generator addresses this issue by introducing dynamic binding — the delegation/inclusion happens at permission evaluation time. If the RDM policy were defined as follows:

class RDMPermissionPolicy:
    can_manage = [
        RecordOwners(),
        RecordCommunitiesAction("curate"),
        AccessGrant("manage"),
        SystemProcess(),
    ]
    can_curate = SameAs("can_manage") + [AccessGrant("edit"), SecretLinks("edit")]

the example above would work as the user would intuitively expect.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

• I'm aware of the code of conduct.
• I've created logical separate commits and followed the commit message format.
• I've added relevant test cases.
• I've added relevant documentation.
• I've marked translation strings.
• I've identified the copyright holder(s) and updated copyright headers for touched files (>15 lines contributions).
• I've NOT included third-party code (copy/pasted source code or new dependencies).
  • If you have added third-party code (copy/pasted or new dependencies), please reach out to an architect on Discord.

Frontend

• I've followed the CSS/JS and React guidelines.
• I've followed the web accessibility guidelines.
• I've followed the user interface guidelines.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

1. You license your contribution under the same terms as the current repository’s license.
2. You agree that you have the right to license your contribution under the current repository’s license.

[max-moser]

Tested with our custom permission policy override in a v13 setup, worked great (see the comment on the other PR)!
LGTM

[zzacharo]

@mesemus LGTM, one question about the warning you added about the change being incompatible: Until now the only way to override this was to inherit the corresponding permission class and replicate the static bindings. Can you ellaborate on which cases you could see this breaking for instance administrators?

[mesemus]

@mesemus LGTM, one question about the warning you added about the change being incompatible: Until now the only way to override this was to inherit the corresponding permission class and replicate the static bindings. Can you ellaborate on which cases you could see this breaking for instance administrators?

I agree that this situation would be extremely rare (and probably pathological), but I can still envision the following scenario.

A user created their permission policy in an older version of InvenioRDM, before a new permission (let's call it can_A) was introduced, and they customised a base permission such as can_view. As in this PR the can_A is changed to SameAs, then can_A will now be the modified behaviour of can_view. As a result, can_A allows access to a different group of users than before this change.

One might argue that this effectively "fixes" a misconfiguration in the user's setup. However, it also means that users may gain (or lose) access through can_A without the administrator being aware of it, since its behaviour would now depend on a previously customised permission.

I consider permissions to be "a form" of public API. Because of their importance and stability expectations, changing the effective behaviour of already existing permissions should, in my opinion, require a new major version.

---

Sidenote: I would also like to propagate the SameAs to requests and other permission policies where static binding is used - so we might want to release all the changes together.

[zzacharo]

@mesemus LGTM, one question about the warning you added about the change being incompatible: Until now the only way to override this was to inherit the corresponding permission class and replicate the static bindings. Can you ellaborate on which cases you could see this breaking for instance administrators?

I agree that this situation would be extremely rare (and probably pathological), but I can still envision the following scenario.

A user created their permission policy in an older version of InvenioRDM, before a new permission (let's call it can_A) was introduced, and they customised a base permission such as can_view. As in this PR the can_A is changed to SameAs, then can_A will now be the modified behaviour of can_view. As a result, can_A allows access to a different group of users than before this change.

One might argue that this effectively "fixes" a misconfiguration in the user's setup. However, it also means that users may gain (or lose) access through can_A without the administrator being aware of it, since its behaviour would now depend on a previously customised permission.

I consider permissions to be "a form" of public API. Because of their importance and stability expectations, changing the effective behaviour of already existing permissions should, in my opinion, require a new major version.

Sidenote: I would also like to propagate the SameAs to requests and other permission policies where static binding is used - so we might want to release all the changes together.

@mesemus fine by me, I agree with releasing as major for granular upgrade of instances following the v14dev track :) Ping @ntarocco @slint for reference.

[utnapischtim]

@mesemus LGTM, one question about the warning you added about the change being incompatible: Until now the only way to override this was to inherit the corresponding permission class and replicate the static bindings. Can you ellaborate on which cases you could see this breaking for instance administrators?

I agree that this situation would be extremely rare (and probably pathological), but I can still envision the following scenario.

A user created their permission policy in an older version of InvenioRDM, before a new permission (let's call it can_A) was introduced, and they customised a base permission such as can_view. As in this PR the can_A is changed to SameAs, then can_A will now be the modified behaviour of can_view. As a result, can_A allows access to a different group of users than before this change.

One might argue that this effectively "fixes" a misconfiguration in the user's setup. However, it also means that users may gain (or lose) access through can_A without the administrator being aware of it, since its behaviour would now depend on a previously customised permission.

I consider permissions to be "a form" of public API. Because of their importance and stability expectations, changing the effective behaviour of already existing permissions should, in my opinion, require a new major version.

Sidenote: I would also like to propagate the SameAs to requests and other permission policies where static binding is used - so we might want to release all the changes together.

i try to understand the scenario. so this would mean that a instance manager has as an example set up their v13 instance with the following custom permission policy

class MyCustomPermissionPolicy(RDMRecordPermissionPolicy):
    can_view = [MyGenerator]
    can_A = can_view

    # Allow reading metadata of a record
    # can_read = [
    #    IfRestricted("record", then_=can_view, else_=can_all),
    # ]
    # since can_read is not overriden can_view is used from the parent
    

as i understand the inheritance can_view in can_read is still from RDMRecordPermissionPolicy which would mean that only can_A will get the can_view from MyCustomPermissionPolicy

does this mean that since now can_read looks like

    can_read = [
        IfRestricted("record", then_=SameAs("can_view"), else_=SameAs("can_all")),
    ]

SameAs looks up the can_view of MyCustomPermissionPolicy and therefore the updated can_view is not more applied only MyCustomPermissionPolicy but to the inherited class properties like can_read too?

which by definition is the main feature of SameAs.

[mesemus]

I was trying to find an example that would not be too pathological :) Let's try to illustrate it on a pair of can_review and can_publish (and @utnapischtim sorry for leading you astray with a permission A while I could have used already existing ones):

In invenio_rdm_records, we've had in RDM13 (with a bit of simplification):

class RDMRecordPermissionPolicy:
   can_review = [...]
   can_publish = can_review    # IfConfig there but can_review is used in the end

Now let's suppose repository administrator has created the following policy:

class ExtendedRecordPermissionPolicy(RDMRecordPermissionPolicy):
    can_review = (
        [UserWithRole('extra-review-role')] +
        RDMRecordPermissionPolicy.can_review
    )

In RDM13, this results in users with extra-review-role being able to modify the draft record (changing metadata, adding files, etc.) but unable to publish - which might have been exactly what the administrator intended.

With this PR, can_publish = SameAs("can_review"), meaning users with extra-review-role suddenly gain access to the publish actions while the repository administrator remains unaware of this change -- unless we make it extremely clear somewhere.

[utnapischtim]

@mesemus so we have this: https://xkcd.com/1172/ situation?

and thanks for the explanation. it's clear now

[mesemus]

@mesemus so we have this: https://xkcd.com/1172/ situation?

Yes - one way repository administrators may have extended the permission policy was by copying the entire RDM policy. Those installations are safe with this change, but administrators should still be informed that there is now a better way.

Another approach admins might have taken would be to copy/override only the parts they really wanted to change, not the whole policy - like in the example above. With this PR, installations that followed this approach might experience altered access behaviour.

Btw, I've also propagated the SameAs to communities and requests - see inveniosoftware/invenio-requests#582
and inveniosoftware/invenio-communities#1355