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

Switch to permissions only #22

Merged
merged 3 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -187,18 +187,29 @@ Permit the moderators to view the draft under review
----------------------------------------------------

For curation reviews to make sense, it is of course vital for the moderators to be able to view the drafts in question.
Per default, `Invenio-Curations` will create access grants for users with the moderation role as part of the curation requests.
This should work out of the box without any further configuration needed.
However, it has the downside of creating additional artifacts in the system, and users could accidentally revoke access for moderators by revoking this access grant.

If this is a deal-breaker for you, there is still the alternative of configuring the records permission policy, similar to the requests permission policy above.
`Invenio-Curations` offers two permission generators that can come in handy for this purpose: ``CurationModerators`` and ``IfCurationRequestExists``.
The former creates ``RoleNeed`` for the configured ``CURATIONS_MODERATION_ROLE``.
It is intended to be used together with the latter, which checks if an ``rdm-curation`` request exists for the given record/draft.

Because the second approach makes access grants unnecessary, their creation can be disabled by setting ``CURATIONS_PERMISSIONS_VIA_GRANTS = False``.
However, please note that overriding the permission policy for records is significantly more complex than overriding the one for requests!
In fact, it's out of scope for this README.
In fact, it's out of scope for this README - or is it?
Copy link
Collaborator

Choose a reason for hiding this comment

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

image



Set RDM permission policy
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Reasons to not rely on access grants:
- They can be completely disabled for an instance
- They can be managed by users which means they can just remove access for the moderators

Thus, we provide a very basic adaptation of the RDM record permission policy used in a vanilla instance. This adapted policy should serve as
an easy way to test the package as well as provide a starting point to understand which permissions have to be adapted for this module to work as expected.

.. code-block:: python

from invenio_curations.services.permissions import CurationRDMRecordPermissionPolicy
RDM_PERMISSION_POLICY = CurationRDMRecordPermissionPolicy


Make the new workflow available through the UI
Expand Down
6 changes: 0 additions & 6 deletions invenio_curations/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@
}
"""Invenio requests facets."""

CURATIONS_PERMISSIONS_VIA_GRANTS = True
"""Share access to records by creating access grants for records under review.

This eliminates the requirement for overriding the record permission policy,
but introduces additional artifacts in the system for each record.
"""

CURATIONS_ALLOW_PUBLISHING_EDITS = False
"""Allow publishing of metadata edits for already published records.
Expand Down
40 changes: 0 additions & 40 deletions invenio_curations/requests/curation.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,8 @@

"""Curation request type."""

from invenio_access.permissions import system_identity
from invenio_drafts_resources.services.records.uow import ParentRecordCommitOp
from invenio_i18n import lazy_gettext as _
from invenio_notifications.services.uow import NotificationOp
from invenio_rdm_records.services.errors import GrantExistsError
from invenio_requests.customizations import RequestState, RequestType, actions

from invenio_curations.notifications.builders import (
Expand All @@ -22,49 +19,12 @@
CurationRequestSubmitNotificationBuilder,
)

from ..proxies import current_curations_service


class CurationCreateAndSubmitAction(actions.CreateAndSubmitAction):
"""Create and submit a request."""

def execute(self, identity, uow):
"""Execute the create & submit action."""
receiver = self.request.receiver.resolve()
record = self.request.topic.resolve()

# if configured, share access to the record with moderators by creating grants
# rather than requiring an override of the record permission policy
if current_curations_service.moderator_permissions_via_grants:
data = {
"grants": [
{
"permission": "preview",
"subject": {
"type": "role",
"id": str(receiver.id),
},
"origin": f"request:{self.request.id}",
}
]
}

service = self.request.topic.get_resolver().get_service()
# NOTE: we're using the system identity here to avoid the grant creation
# potentially being blocked by the requesting user's profile visibility
try:
service.access.bulk_create_grants(
system_identity, record.pid.pid_value, data
)
except GrantExistsError:
pass

uow.register(
ParentRecordCommitOp(
record.parent, indexer_context=dict(service=service)
)
)

uow.register(
NotificationOp(
CurationRequestSubmitNotificationBuilder.build(
Expand Down
4 changes: 2 additions & 2 deletions invenio_curations/services/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from invenio_curations.services import facets

from .permissions import CurationPermissionPolicy
from .permissions import CurationRDMRequestPermissionPolicy


class CurationsSearchOptions(RequestSearchOptions):
Expand Down Expand Up @@ -43,7 +43,7 @@ class CurationsServiceConfig(RecordServiceConfig, ConfiguratorMixin):

# common configuration
permission_policy_cls = FromConfig(
"REQUESTS_PERMISSION_POLICY", default=CurationPermissionPolicy
"REQUESTS_PERMISSION_POLICY", default=CurationRDMRequestPermissionPolicy
)
# TODO: update search options?
search = CurationsSearchOptions
61 changes: 59 additions & 2 deletions invenio_curations/services/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,71 @@

"""Curations permissions."""

from invenio_rdm_records.services.generators import IfFileIsLocal
from invenio_rdm_records.services.permissions import RDMRecordPermissionPolicy
from invenio_records_permissions.generators import SystemProcess
from invenio_requests.services.generators import Creator, Receiver, Status
from invenio_requests.services.permissions import (
PermissionPolicy as RequestPermissionPolicy,
)

from invenio_curations.services.generators import (
CurationModerators,
IfCurationRequestExists,
)


class CurationRDMRecordPermissionPolicy(RDMRecordPermissionPolicy):
"""RDM record policy for curations."""

can_preview = RDMRecordPermissionPolicy.can_preview + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]
can_view = RDMRecordPermissionPolicy.can_view + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]
can_read = RDMRecordPermissionPolicy.can_read + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]
can_read_files = RDMRecordPermissionPolicy.can_read_files + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]

# in order to get all base permissions in, we just add ours instead of adapting the then_ clause of the base permission
can_get_content_files = RDMRecordPermissionPolicy.can_get_content_files + [
IfFileIsLocal(then_=can_read_files, else_=[SystemProcess()])
]

can_read_draft = RDMRecordPermissionPolicy.can_read_draft + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]
can_draft_read_files = RDMRecordPermissionPolicy.can_draft_read_files + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]

# in order to get all base permissions in, we just add ours instead of adapting the then_ clause of the base permission
can_draft_get_content_files = (
RDMRecordPermissionPolicy.can_draft_get_content_files
+ [IfFileIsLocal(then_=can_draft_read_files, else_=[SystemProcess()])]
)

# in order to get all base permissions in, we just add ours instead of adapting the then_ clause of the base permission
can_draft_media_get_content_files = (
RDMRecordPermissionPolicy.can_draft_media_get_content_files
+ [IfFileIsLocal(then_=can_preview, else_=[SystemProcess()])]
)

can_media_read_files = RDMRecordPermissionPolicy.can_media_read_files + [
IfCurationRequestExists(then_=[CurationModerators()], else_=[])
]
can_media_get_content_files = (
RDMRecordPermissionPolicy.can_media_get_content_files
+ [IfFileIsLocal(then_=can_read, else_=[SystemProcess()])]
)
Comment on lines +25 to +71
Copy link
Collaborator

Choose a reason for hiding this comment

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

image



class CurationPermissionPolicy(RequestPermissionPolicy):
"""Permission policy for curations."""
class CurationRDMRequestPermissionPolicy(RequestPermissionPolicy):
"""Request permission policy for curations."""

can_read = RequestPermissionPolicy.can_read + [
Status(
Expand Down
5 changes: 0 additions & 5 deletions invenio_curations/services/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ def allow_publishing_edits(self):
"""Get the configured value of ``CURATIONS_ALLOW_PUBLISHING_EDITS``."""
return current_app.config.get("CURATIONS_ALLOW_PUBLISHING_EDITS", False)

@property
def moderator_permissions_via_grants(self):
"""Get the configured value of ``CURATIONS_PERMISSIONS_VIA_GRANTS``."""
return current_app.config.get("CURATIONS_PERMISSIONS_VIA_GRANTS", True)

@property
def moderation_role_name(self):
"""Get the configured name of the ``CURATIONS_MODERATION_ROLE``."""
Expand Down