Skip to content

Conversation

@xsenyaaax
Copy link

Description

At CESNET we extend requests' services to have them as first class citizens (we have REST API to create requests in a generic way). In order to do so, we need to extend permission checks so that they not only include requests but also create/update data and additional parameters from context.

This pull request:

  • add kwargs to service methods
  • passes the kwargs data and additional context to permission checks

This change is 100% backwards compatible and in RDM does not cause any differences in permission handling

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:

  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.

identity,
"create_comment",
request=request,
data=data,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you run specific permission checks on comment actions based on request and event_type? I am trying to understand if we don't overpass kwargs down to require_permission. Also, I would be interested to know on how you use them in your use cases :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Zach,

General intro (sorry, it will be a bit longer):

What we do in our overlay is that each community can have one or more associated workflows (one of them being "default"). The workflow handles the exact deposition/record lifecycle, regardless of whether the record is in draft or published state.

When a record is created within a community (we don't allow creating records outside of a community), we take the default workflow from the community and set a workflow attribute (string) on the record's parent. This workflow then identifies the exact deposition/record lifecycle workflow.

Additionally, each record has a state field (initially set to draft), and record-level permissions use it for checks.

Workflows are defined in Python (invenio.cfg) as a combination of record permissions and requests permissions:

WORKFLOWS = {
    "default": Workflow(
        label=_("Default workflow for normal communities"),
        permission_policy_cls=DefaultWorkflowPermissions,
        request_policy_cls=DefaultWorkflowRequests,
    ),
    ...
}

Example from the record permission policy part:

# piece of DefaultWorkflowPermissions

can_read = [
      IfInState(
          "published",
          then_=[
              IfRestricted(
                  "record",
                  then_=[PrimaryCommunityMembers()],
                  else_=[AnyUser()],
              )
          ],
      ),
      IfInState(
          "retracting",
          then_=[
              RecordOwner(),
              PrimaryCommunityRole("curator")
          ],
      ),
]

When permission is checked on a record (for example, can_read), the record's permission policy will look at the record's parent, find the workflow ID, resolve it to the record permission policy associated with the workflow, and execute can_read on it. There, IfInState looks at the state field in the record metadata, and if the record is in retracting state, only the record owner and curator are given access.

Now, requests

The second part of the workflow defines who can create requests, when they can create them, what events can occur on those requests, who can accept/decline them, etc. It also defines escalations if the request isn't handled within a specified time interval.

Example:

class GenericCommunityWorkflowRequests(WorkflowRequestPolicy):
    delete_published_record = WorkflowRequest(
        requesters=[
            IfNotHarvested(
                then_=IfInState(
                    "published",
                    then_=[
                        RecordOwners(),
                        PrimaryCommunityRole("curator"),
                        PrimaryCommunityRole("owner"),
                    ],
                ),
                else_=[SystemProcess()],
            )
        ],
        recipients=[
            IfRequestedBy(
                requesters=[
                    PrimaryCommunityRole("curator"),
                    PrimaryCommunityRole("owner"),
                ],
                then_=[AutoApprove()],
                else_=[PrimaryCommunityRole("curator")],
            )
        ],
        transitions=WorkflowTransitions(
            submitted="retracting",
            declined="published",
            accepted="deleted",
            cancelled="published",
        ),
        events={
            "event_type": WorkflowEvent(
                submitters=[
                    # generators that can use the topic, request, request_type, event_type
                ],
            )
        }
        escalations=[
            WorkflowRequestEscalation(
                after=timedelta(days=21),
                recipients=[
                    PrimaryCommunityRole("owner"),
                ],
            )
        ],
    )

Imagine that the owner of /api/records/abc would like to delete the record. With workflows, the user directly creates a request for that:

POST /api/records/abc/requests/delete_published_record

{...potentially some payload here, such as deletion reason...}

This in turn calls Invenio Request Service's create method, passing the payload and additionally the abc record in kwargs.

We've replaced the can_create permission of the Invenio Requests Service with:

    can_create = [
        SystemProcess(),
        RequestCreatorsFromWorkflow(),
    ]

For the POST above: RequestCreatorsFromWorkflow takes the abc record from the kwargs, looks up its workflow, finds delete_published_record in the workflow's requests configuration, and uses the requesters field.

If the user has permission, it will use the recipients property from the configuration above to determine who should be placed in the recipient field of the newly created request. An AutoApprove above is a special case - the request is accepted automatically.

When the request is submitted, the transitions part will automatically change the state field on the record to retracting, causing that only the owner and curator will have access to the record at this stage.

Finally, the escalations part will check in a Celery task that the request hasn't been handled for x days, will reassign the recipient of the request to a new person, and send notifications.

For events, it works the same way, using the submitters on the WorkflowEvent. We use different event types (not just comment events) during the lifecycle of the request - for example, if we have a repository with sensitive data where access to files is controlled by a data access committee, we'll use a special event type to record the committee's decision in a structured way.

Final note: we are not the ones that write the workflow policies - it is the owners of the repositories built on our overlay. That's why we would like to pass as complete a context to permissions (and components as well) as possible to enable them writing their possibly complex permission generators.

So that's why we need to have those kwargs and extra context :) We've been using this from our own fork for the last year or so. If it gets into Invenio main, we'll be able to remove our fork. If it doesn't fit RDM (for example, for security/API compactness considerations), I understand - no harm done, we'll just keep our patch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mesemus thanks a lot for the amazing explanation! I see you have some really advanced customization which might be also interesting for the core. I understand now a bit better your use case but now I am intrigued to know a bit more (maybe a demo?) of your use case examples. Have you also changed the UI to accomodate all these different deposition workflows?

I am interested because I have started investigating (we are in the phase of conducting some interviews with our CERN stakeholders) potential solutions/improvements on the publication workflow we support currently on RDM. For example, we are working now to add "Reviewers" as a first class citizen of the request and in parallel thinking on how to improve the commenting capabilities on a request.

I will be off until 22nd of April but I would be interested to see in a demo your use case and consider if there is any common pathway to integrate fully or partially the customizable publication workflows.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zzacharo sure, let's move this to Discord to find a time slot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

3 participants