[GRDM-62315, 60282] mAP core グループ権限の対応漏れ / タイトルが長いプロジェクトでワークフローを開始できない の修正

addons/workflow/services.py

@@ -216,7 +216,7 @@ def get_user_accessible_templates(user: OSFUser, **filters) -> List[WorkflowTemp
     Returns:
         List of WorkflowTemplate objects the user can access
     """
-    accessible_nodes = AbstractNode.objects.filter(_contributors=user, is_deleted=False)
+    accessible_nodes = AbstractNode.objects.get_nodes_for_user(user, include_mapcore_groups=True)
 
     visibility_filter = Q(pk__in=[])
     visibility_filter |= Q(visibility=WorkflowTemplate.VISIBILITY_PUBLIC)

addons/workflow/tests/test_template_access.py

@@ -0,0 +1,165 @@
+# -*- coding: utf-8 -*-
+"""Property tests for the template access predicate.
+
+The node-membership branch of template/engine access must admit exactly the
+users who hold an explicit role on a related node: contributors and members
+of mAP core groups bound to the node. Implicit parent-admin READ and public
+visibility of the node itself grant nothing through this branch.
+"""
+
+import uuid
+
+import pytest
+
+from framework.auth.core import Auth
+from framework.exceptions import HTTPError
+
+from osf.models.mapcore_group import MapCoreGroup
+from osf.models.mapcore_node_group import MapCoreNodeGroup
+from osf.models.mapcore_user_group import MapCoreUserGroup
+from osf.utils import permissions
+from osf_tests.factories import (
+    AuthUserFactory,
+    InstitutionFactory,
+    NodeFactory,
+    ProjectFactory,
+)
+from tests.base import OsfTestCase
+
+from addons.workflow.models import (
+    WorkflowActivation,
+    WorkflowDefinitionSnapshot,
+    WorkflowEngine,
+    WorkflowTemplate,
+)
+from addons.workflow.services import get_user_accessible_templates
+from addons.workflow.views import _get_engine_or_404, _get_template_or_404
+
+pytestmark = pytest.mark.django_db
+
+
+class TemplateAccessPredicateTests(OsfTestCase):
+    def setUp(self):
+        super().setUp()
+        self.owner = AuthUserFactory()
+        self.node = ProjectFactory(creator=self.owner)
+        institution = InstitutionFactory()
+        self.engine = WorkflowEngine.objects.create(
+            engine_id=str(uuid.uuid4()),
+            gateway_base_url='https://workflow.example/api/',
+            signing_kid='kid-test',
+            institution=institution,
+        )
+        snapshot = WorkflowDefinitionSnapshot.objects.create(
+            engine=self.engine,
+            definition_id='definition-access',
+            definition_key='definition-access',
+            name='Access Process',
+            version=1,
+        )
+        self.template = WorkflowTemplate.objects.create(
+            node=self.node,
+            definition=snapshot,
+            registered_by=self.owner,
+        )
+
+    def _bind_mapcore_group(self, node, user, permission=permissions.WRITE, enable_addon=True):
+        if enable_addon:
+            node.add_addon('groups', auth=Auth(node.creator))
+        mapcore_group = MapCoreGroup.objects.create(_id='group-{}'.format(uuid.uuid4()))
+        MapCoreNodeGroup.objects.create(
+            node=node,
+            group=node.get_group(permission),
+            mapcore_group=mapcore_group,
+            creator=node.creator,
+        )
+        MapCoreUserGroup.objects.create(user=user, mapcore_group=mapcore_group)
+
+    def _assert_has_access(self, user, template=None, engine=None):
+        template = template or self.template
+        engine = engine or self.engine
+        assert _get_template_or_404(template.pk, user) == template
+        assert _get_engine_or_404(engine.engine_id, user) == engine
+        assert template in get_user_accessible_templates(user)
+
+    def _assert_denied(self, user, template=None, engine=None):
+        template = template or self.template
+        engine = engine or self.engine
+        with pytest.raises(HTTPError) as excinfo:
+            _get_template_or_404(template.pk, user)
+        assert excinfo.value.code == 404
+        with pytest.raises(HTTPError) as excinfo:
+            _get_engine_or_404(engine.engine_id, user)
+        assert excinfo.value.code == 404
+        assert template not in get_user_accessible_templates(user)
+
+    def test_contributor_has_access(self):
+        contributor = AuthUserFactory()
+        self.node.add_contributor(contributor, permissions=permissions.READ, auth=Auth(self.owner), save=True)
+        self._assert_has_access(contributor)
+
+    def test_mapcore_group_member_has_access(self):
+        member = AuthUserFactory()
+        self._bind_mapcore_group(self.node, member)
+        self._assert_has_access(member)
+
+    def _assert_activation_access(self, user):
+        assert _get_template_or_404(self.template.pk, user) == self.template
+        assert _get_engine_or_404(self.engine.engine_id, user) == self.engine
+        # get_user_accessible_templates only lists templates hosted on the
+        # user's own nodes; activation-node members are out of its scope
+        assert self.template not in get_user_accessible_templates(user)
+
+    def test_contributor_has_access_via_activation(self):
+        contributor = AuthUserFactory()
+        activation_node = ProjectFactory()
+        activation_node.add_contributor(contributor, permissions=permissions.READ, auth=Auth(activation_node.creator), save=True)
+        WorkflowActivation.objects.create(
+            node=activation_node,
+            template=self.template,
+            activated_by=self.owner,
+        )
+        self._assert_activation_access(contributor)
+
+    def test_mapcore_group_member_has_access_via_activation(self):
+        member = AuthUserFactory()
+        activation_node = ProjectFactory()
+        self._bind_mapcore_group(activation_node, member)
+        WorkflowActivation.objects.create(
+            node=activation_node,
+            template=self.template,
+            activated_by=self.owner,
+        )
+        self._assert_activation_access(member)
+
+    def test_parent_admin_denied(self):
+        parent_admin = AuthUserFactory()
+        parent = ProjectFactory(creator=parent_admin)
+        component = NodeFactory(parent=parent, creator=self.owner)
+        snapshot = WorkflowDefinitionSnapshot.objects.create(
+            engine=self.engine,
+            definition_id='definition-component',
+            definition_key='definition-component',
+            name='Component Process',
+            version=1,
+        )
+        template = WorkflowTemplate.objects.create(
+            node=component,
+            definition=snapshot,
+            registered_by=self.owner,
+        )
+        assert component.has_permission(parent_admin, permissions.READ)
+        self._assert_denied(parent_admin, template=template)
+
+    def test_stranger_denied_on_public_node(self):
+        self.node.is_public = True
+        self.node.save()
+        self._assert_denied(AuthUserFactory())
+
+    def test_unrelated_user_denied(self):
+        self._assert_denied(AuthUserFactory())
+
+    def test_group_member_denied_when_groups_addon_disabled(self):
+        member = AuthUserFactory()
+        self._bind_mapcore_group(self.node, member, enable_addon=False)
+        self._assert_denied(member)

addons/workflow/tests/test_token.py

@@ -0,0 +1,48 @@
+# -*- coding: utf-8 -*-
+
+import pytest
+
+from addons.workflow.token import MAX_TOKEN_NAME_LENGTH, build_token_name
+
+
+class TestBuildTokenName:
+    def test_no_label(self):
+        name = build_token_name('executor')
+        assert name == 'Workflow delegation: executor'
+
+    def test_short_label(self):
+        name = build_token_name('executor', 'test on proj')
+        assert name == 'Workflow delegation: executor (test on proj)'
+        assert len(name) <= MAX_TOKEN_NAME_LENGTH
+
+    def test_exact_boundary(self):
+        # 'Workflow delegation: executor' = 29 chars
+        # suffix = ' (' + label + ')' = len(label) + 3
+        # total = 32 + len(label) = 100 => len(label) = 68
+        label = 'x' * 68
+        name = build_token_name('executor', label)
+        assert len(name) == MAX_TOKEN_NAME_LENGTH
+        assert '...' not in name
+
+    def test_one_over_truncates(self):
+        label = 'x' * 69
+        name = build_token_name('executor', label)
+        assert len(name) == MAX_TOKEN_NAME_LENGTH
+        assert name.endswith('...)')
+
+    def test_very_long_label(self):
+        label = 'x' * 300
+        name = build_token_name('executor', label)
+        assert len(name) == MAX_TOKEN_NAME_LENGTH
+        assert name.endswith('...)')
+
+    def test_japanese_label(self):
+        label = '論文・根拠データ公開申請(ファイル選択) on ' + 'あ' * 100
+        name = build_token_name('executor', label)
+        assert len(name) <= MAX_TOKEN_NAME_LENGTH
+        assert name.startswith('Workflow delegation: executor (')
+
+    def test_all_roles(self):
+        for role in ('creator', 'manager', 'executor'):
+            name = build_token_name(role, 'a' * 200)
+            assert len(name) <= MAX_TOKEN_NAME_LENGTH

addons/workflow/token.py

@@ -16,6 +16,20 @@
 ALLOWED_TOKEN_ROLES = frozenset({'creator', 'manager', 'executor'})
 REQUIRED_DELEGATION_FIELDS = frozenset({'token_id', 'token_value', 'scope', 'token_owner'})
 
+MAX_TOKEN_NAME_LENGTH = 100
+
+
+def build_token_name(role: str, label: str = '') -> str:
+    token_name = f'Workflow delegation: {role}'
+    if label:
+        suffix = f' ({label})'
+        max_suffix = MAX_TOKEN_NAME_LENGTH - len(token_name)
+        if len(suffix) > max_suffix:
+            # ' (' = 2, '...)' = 4, total overhead = 6
+            suffix = f' ({label[:max_suffix - 6]}...)'
+        token_name += suffix
+    return token_name
+
 TOKEN_MODE_TO_SCOPE = {
     'read': ['osf.full_read', 'osf.users.email_read', 'osf.users.profile_read'],
     'readwrite': ['osf.full_read', 'osf.full_write', 'osf.users.email_read', 'osf.users.profile_read'],
@@ -175,9 +189,7 @@ def create_delegation_token(user, role: str, mode: str, label: str = '') -> Dict
             )
         scopes.append(scope)
 
-    token_name = f'Workflow delegation: {role}'
-    if label:
-        token_name += f' ({label})'
+    token_name = build_token_name(role, label)
 
     token = ApiOAuth2PersonalToken(
         owner=user,

addons/workflow/views.py

@@ -246,15 +246,14 @@ def _get_engine_or_404(engine_id: str, user) -> WorkflowEngine:
 
     has_institution_access = _user_has_engine_admin_access(user, engine)
     if not has_institution_access:
-        accessible_nodes = AbstractNode.objects.filter(_contributors=user, is_deleted=False)
+        accessible_nodes = AbstractNode.objects.get_nodes_for_user(user, include_mapcore_groups=True)
         has_template_access = WorkflowTemplate.objects.filter(
             node__in=accessible_nodes,
             definition__engine=engine,
         ).exists()
         if not has_template_access:
             has_template_access = WorkflowActivation.objects.filter(
-                node___contributors=user,
-                node__is_deleted=False,
+                node__in=accessible_nodes,
                 template__definition__engine=engine,
             ).exists()
 
@@ -293,12 +292,11 @@ def _get_template_or_404(template_id: str, user) -> WorkflowTemplate:
             data={'message': 'Workflow template not found.'},
         )
 
-    has_direct_access = template.node.contributors.filter(id=user.id).exists()
+    has_direct_access = template.node.is_contributor_or_group_member(user)
     if not has_direct_access:
         has_activation_access = WorkflowActivation.objects.filter(
             template=template,
-            node___contributors=user,
-            node__is_deleted=False,
+            node__in=AbstractNode.objects.get_nodes_for_user(user, include_mapcore_groups=True),
         ).exists()
         if not has_activation_access and not _user_can_access_template_via_visibility(user, template):
             raise HTTPError(

api/nodes/utils.py

@@ -12,6 +12,7 @@
 from addons.wiki.models import NodeSettings as WikiNodeSettings
 from osf.models import AbstractNode, Preprint, Guid, NodeRelation, Contributor
 from osf.models.node import NodeGroupObjectPermission
+from osf.models.mapcore_node_group import MapCoreNodeGroup
 from osf.utils import permissions
 
 from api.base.exceptions import ServiceUnavailableError
@@ -98,7 +99,23 @@ def optimize_node_queryset(self, queryset):
         read_permission = Permission.objects.get(codename=permissions.READ_NODE)
         contrib = Contributor.objects.filter(user=auth.user, node=OuterRef('pk'))
         user_group = OSFUserGroup.objects.filter(osfuser_id=auth.user.id if auth.user else None, group_id=OuterRef('group_id'))
-        node_group = NodeGroupObjectPermission.objects.annotate(user_group=Subquery(user_group.values_list('group_id')[:1])).filter(user_group__isnull=False, content_object_id=OuterRef('pk'))
+        # mAP core group membership grants node permissions through the node's
+        # permission auth groups without an osfuser_groups row, so it needs its
+        # own membership path here (mirrors AbstractNode.get_permissions).
+        mapcore_node_group = MapCoreNodeGroup.objects.filter(
+            is_deleted=False,
+            group_id=OuterRef('group_id'),
+            mapcore_group__mapcore_user_groups__user_id=auth.user.id if auth.user else None,
+            mapcore_group__mapcore_user_groups__is_deleted=False,
+            node__addons_groups_node_settings__is_deleted=False,
+        )
+        node_group = NodeGroupObjectPermission.objects.annotate(
+            user_group=Subquery(user_group.values_list('group_id')[:1]),
+            mapcore_member=Exists(mapcore_node_group),
+        ).filter(
+            Q(user_group__isnull=False) | Q(mapcore_member=True),
+            content_object_id=OuterRef('pk'),
+        )
         # user_is_contrib means user is a traditional contributor, while has_read/write/admin are permissions the user has either through group membership or contributorship
         return queryset.prefetch_related('root').prefetch_related('subjects').annotate(
             user_is_contrib=Exists(contrib),

api_tests/users/views/test_user_nodes_mapcore_group_permissions.py

@@ -0,0 +1,58 @@
+import pytest
+
+from api.base.settings.defaults import API_BASE
+from framework.auth.core import Auth
+from osf.models.mapcore_group import MapCoreGroup
+from osf.models.mapcore_node_group import MapCoreNodeGroup
+from osf.models.mapcore_user_group import MapCoreUserGroup
+from osf_tests.factories import (
+    AuthUserFactory,
+    ProjectFactory,
+)
+from osf.utils import permissions
+
+
+@pytest.mark.django_db
+class TestUserNodesMapCoreGroupPermissions:
+    """current_user_permissions on the nodes list must reflect mAP core
+    group membership, matching the node detail endpoint."""
+
+    @pytest.fixture()
+    def group_member(self):
+        return AuthUserFactory()
+
+    @pytest.fixture()
+    def project(self, group_member):
+        creator = AuthUserFactory()
+        project = ProjectFactory(creator=creator, is_public=False)
+        project.add_addon('groups', auth=Auth(creator))
+        project.save()
+
+        mapcore_group = MapCoreGroup.objects.create(_id='test-mapcore-group')
+        MapCoreNodeGroup.objects.create(
+            node=project,
+            group=project.get_group(permissions.WRITE),
+            mapcore_group=mapcore_group,
+            creator=creator,
+        )
+        MapCoreUserGroup.objects.create(
+            user=group_member,
+            mapcore_group=mapcore_group,
+        )
+        return project
+
+    def test_list_permissions_match_detail(self, app, group_member, project):
+        list_url = '/{}users/me/nodes/'.format(API_BASE)
+        res = app.get(list_url, auth=group_member.auth)
+        assert res.status_code == 200
+        nodes = {n['id']: n for n in res.json['data']}
+        assert project._id in nodes
+
+        detail_url = '/{}nodes/{}/'.format(API_BASE, project._id)
+        detail = app.get(detail_url, auth=group_member.auth)
+        assert detail.status_code == 200
+        detail_perms = detail.json['data']['attributes']['current_user_permissions']
+        assert permissions.WRITE in detail_perms
+
+        list_perms = nodes[project._id]['attributes']['current_user_permissions']
+        assert list_perms == detail_perms