From 44296f752ccd8c43fb26984cd897741b74294b98 Mon Sep 17 00:00:00 2001 From: Ronald Krist Date: Wed, 11 Sep 2024 17:41:34 +0200 Subject: [PATCH 1/2] introducing UserInCommunityNeed and generators RecordOwners generators checking whether Owner is in record's community --- oarepo_communities/ext.py | 12 +- oarepo_communities/services/links.py | 9 +- .../services/permissions/generators.py | 64 ++++++++++ .../services/permissions/policy.py | 5 +- .../ui/oarepo_communities/__init__.py | 59 ++++----- .../ui/oarepo_communities/components.py | 13 +- oarepo_communities/utils.py | 42 ++++++- setup.cfg | 2 +- tests/test_communities/conftest.py | 100 +++++++++++++-- tests/test_communities/test_permissions.py | 119 ++++++++++++++++++ tests/test_communities/utils.py | 8 +- 11 files changed, 375 insertions(+), 58 deletions(-) diff --git a/oarepo_communities/ext.py b/oarepo_communities/ext.py index ae089e3..4e62a32 100644 --- a/oarepo_communities/ext.py +++ b/oarepo_communities/ext.py @@ -1,5 +1,7 @@ from functools import cached_property +from flask_principal import identity_loaded + import oarepo_communities.cli # noqa - imported to register CLI commands from .resources.community_records.config import CommunityRecordsResourceConfig @@ -7,7 +9,7 @@ from .services.community_inclusion.service import CommunityInclusionService from .services.community_records.config import CommunityRecordsServiceConfig from .services.community_records.service import CommunityRecordsService -from .utils import get_urlprefix_service_id_mapping +from .utils import get_urlprefix_service_id_mapping, load_community_user_needs from .workflow import community_default_workflow @@ -24,6 +26,7 @@ def init_app(self, app): self.app = app self.init_services(app) self.init_resources(app) + self.init_hooks(app) self.init_config(app) app.extensions["oarepo-communities"] = self @@ -80,6 +83,13 @@ def init_resources(self, app): service=self.community_records_service, ) + def init_hooks(self, app): + """Initialize hooks.""" + + @identity_loaded.connect_via(app) + def on_identity_loaded(_, identity): + load_community_user_needs(identity) + def get_default_community_from_record(self, record, **kwargs): record = record.parent if hasattr(record, "parent") else record try: diff --git a/oarepo_communities/services/links.py b/oarepo_communities/services/links.py index 7ad740e..e87a898 100644 --- a/oarepo_communities/services/links.py +++ b/oarepo_communities/services/links.py @@ -1,16 +1,17 @@ from copy import deepcopy from typing import Dict -from uritemplate import URITemplate -from invenio_records_resources.services.base.links import preprocess_vars, Link from invenio_communities.communities.records.api import Community +from invenio_records_resources.services.base.links import Link, preprocess_vars +from uritemplate import URITemplate + class CommunitiesLinks(Link): """Utility class for keeping track of and resolve links.""" def __init__(self, uritemplate_strs: Dict, when=None, vars=None): """Constructor.""" - self._uritemplates = {k: URITemplate(v) for k,v in uritemplate_strs.items()} + self._uritemplates = {k: URITemplate(v) for k, v in uritemplate_strs.items()} self._when_func = when self._vars_func = vars @@ -31,4 +32,4 @@ def expand(self, obj, context): link = uritemplate.expand(**vars) community_links[link_name] = link links[community_id] = community_links - return links \ No newline at end of file + return links diff --git a/oarepo_communities/services/permissions/generators.py b/oarepo_communities/services/permissions/generators.py index a982fd9..4c3fd4e 100644 --- a/oarepo_communities/services/permissions/generators.py +++ b/oarepo_communities/services/permissions/generators.py @@ -1,11 +1,14 @@ import abc import uuid +from collections import namedtuple +from functools import partial, reduce from invenio_access.permissions import system_identity from invenio_communities.communities.records.api import Community from invenio_communities.generators import CommunityRoleNeed, CommunityRoles from invenio_communities.proxies import current_communities, current_roles from invenio_records_permissions.generators import Generator +from invenio_search.engine import dsl from oarepo_workflows.errors import MissingWorkflowError from oarepo_workflows.requests.policy import RecipientGeneratorMixin from oarepo_workflows.services.permissions.generators import WorkflowPermission @@ -16,6 +19,9 @@ ) from oarepo_communities.proxies import current_oarepo_communities +_Need = namedtuple("Need", ["method", "user", "community"]) +UserInCommunityNeed = partial(_Need, "user_in_community") + class InAnyCommunity(Generator): def __init__(self, permission_generator, **kwargs): @@ -194,3 +200,61 @@ def roles(self, **kwargs): PrimaryCommunityMembers = DefaultCommunityMembers + + +class RecordOwnerInDefaultRecordCommunity(DefaultCommunityRoleMixin, Generator): + default_or_ids = "default" + + def _record_communities(self, record, **kwargs): + return set(self._get_record_communities(record, **kwargs)) + + def needs(self, record=None, **kwargs): + record_communities = set(self._get_record_communities(record, **kwargs)) + return self._needs(record_communities, record=record) + + def _needs(self, record_communities, record=None): + owners = getattr(record.parent, "owners", None) + ret = [] + for owner in owners: + ret += [ + UserInCommunityNeed(owner.id, community) + for community in record_communities + ] + return ret + + def query_filter(self, identity=None, **kwargs): + """Filters for current identity as owner.""" + user_in_communities = { + (n.user, n.community) + for n in identity.provides + if n.method == "user_in_community" + } + terms = [] + for element in user_in_communities: + terms.append( + dsl.Q("term", **{"parent.owners.user": element[0]}) + & dsl.Q( + "term", **{f"parent.communities.{self.default_or_ids}": element[1]} + ) + ) + if not terms: + return [] + query = reduce(lambda f1, f2: f1 | f2, terms) + return query + + +RecordOwnerInPrimaryRecordCommunity = RecordOwnerInDefaultRecordCommunity + + +class RecordOwnerInRecordCommunity( + CommunityRoleMixin, RecordOwnerInDefaultRecordCommunity +): + default_or_ids = "ids" + + # trick to use CommunityRoleMixin instead of DefaultCommunityRoleMixin + def _record_communities(self, record, **kwargs): + return set(self._get_record_communities(record, **kwargs)) + + def needs(self, record=None, **kwargs): + record_communities = set(self._get_record_communities(record, **kwargs)) + return self._needs(record_communities, record=record) diff --git a/oarepo_communities/services/permissions/policy.py b/oarepo_communities/services/permissions/policy.py index 5c83919..757a825 100644 --- a/oarepo_communities/services/permissions/policy.py +++ b/oarepo_communities/services/permissions/policy.py @@ -1,4 +1,6 @@ -from oarepo_requests.services.permissions.workflow_policies import RequestBasedWorkflowPermissions +from oarepo_requests.services.permissions.workflow_policies import ( + RequestBasedWorkflowPermissions, +) from oarepo_workflows.services.permissions.policy import WorkflowPermissionPolicy from oarepo_communities.services.permissions.generators import ( @@ -21,6 +23,7 @@ class MyWorkflowPermissions(CommunityDefaultWorkflowPermissions): ) } """ + can_create = [ DefaultCommunityMembers(), ] diff --git a/oarepo_communities/ui/oarepo_communities/__init__.py b/oarepo_communities/ui/oarepo_communities/__init__.py index 200aa5b..7d9f19f 100644 --- a/oarepo_communities/ui/oarepo_communities/__init__.py +++ b/oarepo_communities/ui/oarepo_communities/__init__.py @@ -1,43 +1,46 @@ -from oarepo_global_search.ui.config import ( - GlobalSearchUIResourceConfig, - GlobalSearchUIResource, -) -from flask import g -from flask_menu import current_menu +import marshmallow as ma +from flask import g, redirect, url_for from flask_login import login_required -from oarepo_runtime.i18n import lazy_gettext as _ -from flask import redirect, url_for -from flask_resources import resource_requestctx +from flask_menu import current_menu +from flask_resources import from_conf, request_parser, resource_requestctx +from invenio_communities.communities.resources.serializer import ( + UICommunityJSONSerializer, +) from invenio_communities.views.communities import ( - communities_settings as invenio_communities_settings, - communities_settings_pages as invenio_communities_settings_pages, - members as invenio_communities_members, - communities_curation_policy as invenio_communities_curation_policy, communities_about as invenio_communities_about, +) +from invenio_communities.views.communities import ( + communities_curation_policy as invenio_communities_curation_policy, +) +from invenio_communities.views.communities import ( communities_frontpage as invenio_communities_frontpage, - communities_search as invenio_communities_search, +) +from invenio_communities.views.communities import ( communities_new as invenio_communities_new, ) - - +from invenio_communities.views.communities import ( + communities_search as invenio_communities_search, +) +from invenio_communities.views.communities import ( + communities_settings as invenio_communities_settings, +) +from invenio_communities.views.communities import ( + communities_settings_pages as invenio_communities_settings_pages, +) +from invenio_communities.views.communities import members as invenio_communities_members from invenio_communities.views.ui import ( _has_about_page_content, _has_curation_policy_page_content, ) - -from flask_resources import request_parser, from_conf - -from .components import GetCommunityComponent - -from invenio_records_resources.resources.records.resource import ( - request_view_args, -) from invenio_records_resources.proxies import current_service_registry - -from invenio_communities.communities.resources.serializer import ( - UICommunityJSONSerializer, +from invenio_records_resources.resources.records.resource import request_view_args +from oarepo_global_search.ui.config import ( + GlobalSearchUIResource, + GlobalSearchUIResourceConfig, ) -import marshmallow as ma +from oarepo_runtime.i18n import lazy_gettext as _ + +from .components import GetCommunityComponent class CommunityValidationSchema(ma.Schema): diff --git a/oarepo_communities/ui/oarepo_communities/components.py b/oarepo_communities/ui/oarepo_communities/components.py index f261c37..53970b4 100644 --- a/oarepo_communities/ui/oarepo_communities/components.py +++ b/oarepo_communities/ui/oarepo_communities/components.py @@ -1,9 +1,6 @@ -from oarepo_ui.resources.components import UIResourceComponent -from invenio_records_resources.proxies import current_service_registry -from invenio_communities.views.communities import ( - HEADER_PERMISSIONS, -) from flask import request +from invenio_communities.views.communities import HEADER_PERMISSIONS +from oarepo_ui.resources.components import UIResourceComponent class GetCommunityComponent(UIResourceComponent): @@ -18,6 +15,6 @@ def before_ui_search( permissions = community.has_permissions_to(HEADER_PERMISSIONS) extra_context["community"] = community extra_context["permissions"] = permissions - search_options["overrides"]["ui_endpoint"] = ( - f"/communities/{community.id}/records" - ) + search_options["overrides"][ + "ui_endpoint" + ] = f"/communities/{community.id}/records" diff --git a/oarepo_communities/utils.py b/oarepo_communities/utils.py index bc1672f..be7ad14 100644 --- a/oarepo_communities/utils.py +++ b/oarepo_communities/utils.py @@ -1,9 +1,47 @@ -from flask import current_app +from flask import current_app, session from invenio_communities.communities.records.api import Community -from invenio_communities.proxies import current_communities +from invenio_communities.proxies import current_communities, current_identities_cache +from invenio_communities.utils import identity_cache_key from invenio_records_resources.proxies import current_service_registry from oarepo_communities.proxies import current_oarepo_communities +from oarepo_communities.services.permissions.generators import UserInCommunityNeed + + +def get_community_needs_for_identity(identity): + # see invenio_communities.utils.load_community_needs + if identity.id is None: + # no user is logged in + return + + cache_key = identity_cache_key(identity) + community_roles = current_identities_cache.get(cache_key) + if community_roles is None: + # aka Member.get_memberships(identity) + roles_ids = session.get("unmanaged_roles_ids", []) + + member_cls = current_communities.service.members.config.record_cls + managed_community_roles = member_cls.get_memberships(identity) + unmanaged_community_roles = member_cls.get_memberships_from_group_ids( + identity, roles_ids + ) + community_roles = managed_community_roles + unmanaged_community_roles + + current_identities_cache.set( + cache_key, + community_roles, + ) + return community_roles + + +def load_community_user_needs(identity): + # todo assuming there's one user and identity_id = user_id + community_roles = get_community_needs_for_identity(identity) + if not community_roles: + return + communities = {community_role[0] for community_role in community_roles} + needs = {UserInCommunityNeed(identity.id, community) for community in communities} + identity.provides |= needs def get_associated_service(record_service, service_type): diff --git a/setup.cfg b/setup.cfg index 865a6a7..5e32d23 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = oarepo-communities -version = 5.0.14 +version = 5.0.15 description = authors = Ronald Krist readme = README.md diff --git a/tests/test_communities/conftest.py b/tests/test_communities/conftest.py index bb4cdd8..f6a88d8 100644 --- a/tests/test_communities/conftest.py +++ b/tests/test_communities/conftest.py @@ -4,8 +4,8 @@ import pytest import yaml from flask_security import login_user -from invenio_accounts.testutils import login_user_via_session from invenio_access.permissions import system_identity +from invenio_accounts.testutils import login_user_via_session from invenio_app.factory import create_api from invenio_communities.cli import create_communities_custom_field from invenio_communities.communities.records.api import Community @@ -20,19 +20,18 @@ ) from oarepo_requests.receiver import default_workflow_receiver_function from oarepo_requests.services.permissions.generators import RequestActive -from oarepo_requests.services.permissions.workflow_policies import CreatorsFromWorkflowRequestsPermissionPolicy +from oarepo_requests.services.permissions.workflow_policies import ( + CreatorsFromWorkflowRequestsPermissionPolicy, +) from oarepo_runtime.services.permissions import RecordOwners from oarepo_workflows import ( - Workflow, AutoApprove, IfInState, + Workflow, WorkflowRequest, WorkflowRequestPolicy, WorkflowTransitions, ) -from oarepo_communities.services.permissions.policy import ( - CommunityDefaultWorkflowPermissions, -) from thesis.records.api import ThesisDraft from oarepo_communities.proxies import current_oarepo_communities @@ -41,9 +40,13 @@ CommunityMembers, CommunityRole, DefaultCommunityRole, + RecordOwnerInDefaultRecordCommunity, + RecordOwnerInRecordCommunity, TargetCommunityRole, ) - +from oarepo_communities.services.permissions.policy import ( + CommunityDefaultWorkflowPermissions, +) @pytest.fixture() @@ -155,9 +158,37 @@ class TestWithCuratorCommunityWorkflowPermissions(TestCommunityWorkflowPermissio ] +class TestWithRecordOwnerInRecordCommunityWorkflowPermissions( + TestWithCuratorCommunityWorkflowPermissions +): + can_read = [ + RecordOwnerInRecordCommunity(), + CommunityRole("owner"), + IfInState( + "published", + [AnyUser()], + ), + ] + + can_create = TestCommunityWorkflowPermissions.can_create + [AuthenticatedUser()] + + +class TestWithRecordOwnerInDefaultRecordCommunityWorkflowPermissions( + TestWithCuratorCommunityWorkflowPermissions +): + can_read = [ + RecordOwnerInDefaultRecordCommunity(), + CommunityRole("owner"), + IfInState( + "published", + [AnyUser()], + ), + ] + + class DefaultRequests(WorkflowRequestPolicy): publish_draft = WorkflowRequest( - requesters=[IfInState("draft", [RecordOwners()])], + requesters=[IfInState("draft", [RecordOwnerInDefaultRecordCommunity()])], recipients=[DefaultCommunityRole("owner")], transitions=WorkflowTransitions( submitted="publishing", accepted="published", declined="draft" @@ -197,6 +228,26 @@ class DefaultRequests(WorkflowRequestPolicy): ) +class PublishRequestsRecordOwnerInRecordCommunity(DefaultRequests): + publish_draft = WorkflowRequest( + requesters=[IfInState("draft", [RecordOwnerInRecordCommunity()])], + recipients=[DefaultCommunityRole("owner")], + transitions=WorkflowTransitions( + submitted="publishing", accepted="published", declined="draft" + ), + ) + + +class PublishRequestsRecordOwnerInDefaultRecordCommunity(DefaultRequests): + publish_draft = WorkflowRequest( + requesters=[IfInState("draft", [RecordOwnerInDefaultRecordCommunity()])], + recipients=[DefaultCommunityRole("owner")], + transitions=WorkflowTransitions( + submitted="publishing", accepted="published", declined="draft" + ), + ) + + class NoRequests(WorkflowRequestPolicy): publish_draft = WorkflowRequest( requesters=[], @@ -245,6 +296,16 @@ class NoRequests(WorkflowRequestPolicy): permission_policy_cls=TestWithCuratorCommunityWorkflowPermissions, request_policy_cls=DefaultRequests, ), + "record_owner_in_record_community": Workflow( + label=_("For testing RecordOwnerInRecordCommunity generator."), + permission_policy_cls=TestWithRecordOwnerInRecordCommunityWorkflowPermissions, + request_policy_cls=PublishRequestsRecordOwnerInRecordCommunity, + ), + "record_owner_in_default_record_community": Workflow( + label=_("For testing RecordOwnerInDefaultRecordCommunity generator."), + permission_policy_cls=TestWithRecordOwnerInDefaultRecordCommunityWorkflowPermissions, + request_policy_cls=PublishRequestsRecordOwnerInDefaultRecordCommunity, + ), "no": Workflow( label=_("Can't do any requests."), permission_policy_cls=TestCommunityWorkflowPermissions, @@ -276,6 +337,22 @@ def invite(user_id, community_id, role): return invite +@pytest.fixture() +def remover(): + """Add/invite a user to a community with a specific role.""" + + def remove(user_id, community_id): + """Add/invite a user to a community with a specific role.""" + delete_data = { + "members": [{"type": "user", "id": user_id}], + } + member_delete = current_communities.service.members.delete( + system_identity, community_id, delete_data + ) + + return remove + + @pytest.fixture(scope="module") def create_app(instance_path, entry_points): """Application factory fixture.""" @@ -328,7 +405,6 @@ def app_config(app_config): return app_config - @pytest.fixture() def community_reader(UserFixture, app, db, community, inviter): u = UserFixture( @@ -361,13 +437,11 @@ def rando_user(UserFixture, app, db): return u - - - @pytest.fixture(scope="module", autouse=True) def location(location): return location + @pytest.fixture(scope="module") def minimal_community(): """Minimal community metadata.""" @@ -382,6 +456,7 @@ def minimal_community(): }, } + def _community_get_or_create(identity, community_dict, workflow=None): """Util to get or create community, to avoid duplicate error.""" slug = community_dict["slug"] @@ -403,6 +478,7 @@ def community(app, minimal_community, community_owner): community_owner.identity, minimal_community, workflow="default" ) + @pytest.fixture(autouse=True) def init_cf(app, db, cache): from oarepo_runtime.services.custom_fields.mappings import prepare_cf_indices diff --git a/tests/test_communities/test_permissions.py b/tests/test_communities/test_permissions.py index fe33e1e..6b850e9 100644 --- a/tests/test_communities/test_permissions.py +++ b/tests/test_communities/test_permissions.py @@ -1,4 +1,5 @@ import pytest +from invenio_access.permissions import system_identity from invenio_communities.communities.records.api import Community from invenio_communities.proxies import current_communities @@ -171,3 +172,121 @@ def test_can_possibly_create_in_community( record_service.require_permission(community_curator.identity, "view_deposit_page") with pytest.raises(PermissionDeniedError): record_service.require_permission(rando_user.identity, "view_deposit_page") + + +def test_record_owners_in_default_record_community_needs( + community_owner, + community_curator, + logged_client, + community_with_workflow_factory, + inviter, + remover, + service_config, + record_service, + search_clear, +): + # tries to .. in with one community with default workflow allowing reader and owner, than adds another community + # allowing curator, which should allow curator to deposit too + community_1 = community_with_workflow_factory("comm1", community_owner) + community_2 = community_with_workflow_factory("comm2", community_owner) + inviter(community_curator.id, community_1.id, "curator") + inviter(community_curator.id, community_2.id, "curator") + community_curator.identity.provides.add( + CommunityRoleNeed(community_1.id, "curator") + ) + + curator_client = logged_client(community_curator) + + record1 = _create_record_in_community( + curator_client, + community_1.id, + custom_workflow="record_owner_in_default_record_community", + ) + record2 = _create_record_in_community( + curator_client, + community_2.id, + custom_workflow="record_owner_in_default_record_community", + ) + + read_before = curator_client.get(f"/thesis/{record1.json['id']}/draft?expand=true") + search_before = curator_client.get(f"/user/thesis/") + type_ids = { + request_type["type_id"] + for request_type in read_before.json["expanded"]["request_types"] + } + assert "publish_draft" in type_ids + assert len(search_before.json["hits"]["hits"]) == 2 + + remover( + community_curator.id, community_1.id + ) # the curator now isn't in community and should not be able to create + # publish request on the record nor search it + + read_after = curator_client.get(f"/thesis/{record1.json['id']}/draft?expand=true") + assert read_after.status_code == 403 + search_after = curator_client.get(f"/user/thesis/") + assert len(search_after.json["hits"]["hits"]) == 1 + + +def test_record_owners_in_record_community_needs( + community_owner, + community_curator, + logged_client, + community_with_workflow_factory, + community_inclusion_service, + inviter, + remover, + service_config, + record_service, + search_clear, +): + + community_1 = community_with_workflow_factory("comm1", community_owner) + community_2 = community_with_workflow_factory("comm2", community_owner) + community_3 = community_with_workflow_factory("comm3", community_owner) + inviter(community_curator.id, community_1.id, "curator") + inviter(community_curator.id, community_2.id, "curator") + community_curator.identity.provides.add( + CommunityRoleNeed(community_1.id, "curator") + ) + + curator_client = logged_client(community_curator) + + record1 = _create_record_in_community( + curator_client, + community_1.id, + custom_workflow="record_owner_in_record_community", + ) + record2 = _create_record_in_community( + curator_client, + community_3.id, + custom_workflow="record_owner_in_record_community", + ) + + read_before1 = curator_client.get(f"/thesis/{record1.json['id']}/draft?expand=true") + read_before2 = curator_client.get(f"/thesis/{record2.json['id']}/draft?expand=true") + search_before = curator_client.get(f"/user/thesis/") + type_ids1 = { + request_type["type_id"] + for request_type in read_before1.json["expanded"]["request_types"] + } + assert read_before2.status_code == 403 + assert len(search_before.json["hits"]["hits"]) == 1 + assert "publish_draft" in type_ids1 + + community_inclusion_service.include( + record_service.read_draft(system_identity, record2.json["id"])._record, + community_2.id, + record_service=record_service, + default=False, + ) + + read_after = curator_client.get(f"/thesis/{record2.json['id']}/draft?expand=true") + search_after = curator_client.get(f"/user/thesis/") + + type_ids2 = { + request_type["type_id"] + for request_type in read_after.json["expanded"]["request_types"] + } + assert "publish_draft" in type_ids2 + assert len(search_after.json["hits"]["hits"]) == 2 diff --git a/tests/test_communities/utils.py b/tests/test_communities/utils.py index f66a13d..385927d 100644 --- a/tests/test_communities/utils.py +++ b/tests/test_communities/utils.py @@ -8,7 +8,13 @@ def published_record_in_community(client, community_id, record_service, user): return record_item._obj -def _create_record_in_community(client, comm_id): +def _create_record_in_community(client, comm_id, custom_workflow=None): + if custom_workflow: + return client.post( + f"/communities/{comm_id}/thesis", + json={"parent": {"workflow": custom_workflow}}, + ) + return client.post(f"/communities/{comm_id}/thesis", json={}) From d4092a28edc7ce25481bca77056aa5d32cdb211e Mon Sep 17 00:00:00 2001 From: Ronald Krist Date: Fri, 13 Sep 2024 13:22:51 +0200 Subject: [PATCH 2/2] version bump --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 5e32d23..130ed27 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = oarepo-communities -version = 5.0.15 +version = 5.0.16 description = authors = Ronald Krist readme = README.md