fix(security): require workspace membership to read a global view

WorkspaceViewViewSet.retrieve was the only action on the class with no
authorization check, and its queryset supplied none either. It filters on
workspace__slug and then `Q(owned_by=request.user) | Q(access=1)`. That second
clause reads as a visibility predicate but is vacuous: `access` sits in
IssueViewSerializer.read_only_fields so the API never sets it, and the model
defaults it to 1 (Public), so every row matches. Any authenticated account
holding a view id could therefore read any global view in any workspace,
including one it had no membership in.

Requires workspace membership, matching the role set on list().

Also adds regression coverage for IssueDetailIdentifierEndpoint. That endpoint
was reported as missing the guest restriction; it is not — the membership check
at the top of get() is followed, after the issue is fetched, by an explicit
role-5 / guest_view_all_features / created_by check. It had no test, so a guard
preventing a guest from walking PROJ-1..PROJ-N and reading every work item's
description_html was one refactor from being lost silently. Verified
non-vacuous: neutering that check makes the test fail with 200 and the foreign
work item's full payload.

Co-authored-by: Plane AI <noreply@plane.so>

apps/api/plane/app/views/view/base.py

@@ -117,7 +117,17 @@ class WorkspaceViewViewSet(BaseViewSet):
                 return Response(serializer.data, status=status.HTTP_200_OK)
             return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
 
+    @allow_permission(allowed_roles=[ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE")
     def retrieve(self, request, slug, pk):
+        # Requires workspace membership, matching list() above. This was the only
+        # action on this class with no check at all, and get_queryset() does not
+        # supply one either: it filters on workspace__slug and then
+        # `Q(owned_by=request.user) | Q(access=1)`. That second clause looks like
+        # a visibility predicate but is vacuous — `access` is in the
+        # serializer's read_only_fields so the API never sets it, and the model
+        # defaults it to 1 (Public), so every row matches. A caller with no
+        # membership in the workspace could therefore read any global view in it
+        # by id.
         issue_view = self.get_queryset().filter(pk=pk).first()
         serializer = IssueViewSerializer(issue_view)
         recent_visited_task.delay(

apps/api/plane/tests/contract/app/test_undecorated_action_authz_app.py

@@ -0,0 +1,263 @@
+# Copyright (c) 2023-present Plane Software, Inc. and contributors
+# SPDX-License-Identifier: AGPL-3.0-only
+# See the LICENSE file for details.
+
+"""Contract tests for viewset actions whose authorization lives outside a decorator.
+
+These actions are *defined* on their viewset, so a scan for routes falling
+through to a DRF generic mixin cannot see them, and neither carries an
+``@allow_permission`` decorator. That makes them easy to misread in both
+directions — one of the two turned out to be guarded after all.
+
+1. **Fix.** ``WorkspaceViewViewSet.retrieve``
+   (``/workspaces/<slug>/views/<pk>/``) had no check at all, and its queryset
+   supplied none either: it filters on ``workspace__slug`` and then
+   ``Q(owned_by=request.user) | Q(access=1)``. That second clause reads like a
+   visibility predicate but is vacuous — ``access`` sits in the serializer's
+   ``read_only_fields`` so the API never sets it, and the model defaults it to
+   ``1`` (Public), so every row matches. A user with no membership in the
+   workspace could read any global view in it by id. It now requires workspace
+   membership, matching ``list``.
+
+2. **Regression coverage only, no fix.** ``IssueDetailIdentifierEndpoint``
+   (``/workspaces/<slug>/work-items/<PROJ>-<n>/``) resolves a work item by its
+   human-readable sequence number, which makes it an enumeration surface. It was
+   reported as missing the guest restriction. **It is not.** The membership check
+   near the top of ``get`` is followed, after the issue is fetched, by an
+   explicit role-5 / ``guest_view_all_features`` / ``created_by`` check that
+   returns 403. That guard had no test, so the disclosure it prevents was one
+   refactor away from being lost silently. These tests pin it.
+"""
+
+from uuid import uuid4
+
+import pytest
+from rest_framework import status
+from rest_framework.test import APIClient
+
+from plane.db.models import (
+    Issue,
+    IssueView,
+    Project,
+    ProjectMember,
+    User,
+    Workspace,
+    WorkspaceMember,
+)
+
+WORK_ITEM_BY_IDENTIFIER_URL = "/api/workspaces/{slug}/work-items/{project_identifier}-{sequence_id}/"
+WORKSPACE_VIEW_DETAIL_URL = "/api/workspaces/{slug}/views/{pk}/"
+
+
+def _make_user(prefix):
+    """A saved, active user with a unique email."""
+    unique_id = uuid4().hex[:8]
+    user = User.objects.create(
+        email=f"{prefix}-{unique_id}@plane.so",
+        username=f"{prefix}_{unique_id}",
+        first_name=prefix.title(),
+        last_name="User",
+    )
+    user.set_password("test-password")
+    user.save()
+    return user
+
+
+def _client_for(user):
+    client = APIClient()
+    client.force_authenticate(user=user)
+    return client
+
+
+@pytest.fixture
+def project(db, workspace, create_user):
+    """A project with guest_view_all_features at its default of False."""
+    project = Project.objects.create(
+        name="Scoped Project",
+        identifier="SP",
+        workspace=workspace,
+        created_by=create_user,
+    )
+    ProjectMember.objects.create(project=project, member=create_user, workspace=workspace, role=20)
+    return project
+
+
+@pytest.fixture
+def guest(db, workspace, project):
+    """An active project GUEST (role=5)."""
+    user = _make_user("guest")
+    WorkspaceMember.objects.create(workspace=workspace, member=user, role=5)
+    ProjectMember.objects.create(project=project, member=user, workspace=workspace, role=5)
+    return user
+
+
+@pytest.fixture
+def guest_client(guest):
+    return _client_for(guest)
+
+
+def _make_issue(name, project, workspace, author):
+    """Create an issue with a deterministic ``created_by``.
+
+    ``BaseModel.save`` sets ``created_by`` from the current request user, which
+    is anonymous under tests, so a ``created_by=`` kwarg to ``create`` is
+    discarded. Passing ``created_by_id`` to ``save`` sets it explicitly.
+    """
+    issue = Issue(name=name, project=project, workspace=workspace)
+    issue.save(created_by_id=author.id)
+    return issue
+
+
+@pytest.fixture
+def own_issue(db, workspace, project, guest):
+    return _make_issue("Guest's own work item", project, workspace, guest)
+
+
+@pytest.fixture
+def foreign_issue(db, workspace, project, create_user):
+    return _make_issue("Someone else's work item", project, workspace, create_user)
+
+
+@pytest.mark.contract
+class TestWorkItemByIdentifierGuestScope:
+    """A restricted guest must not resolve work items they did not author.
+
+    Pins an existing guard rather than covering a new fix. This endpoint resolves
+    by sequence number, so losing the guard would let a guest walk
+    ``PROJ-1..PROJ-N`` and read every work item's full detail — including
+    ``description_html`` — plus the project and issue UUIDs that every other
+    endpoint keys on.
+    """
+
+    @pytest.mark.django_db
+    def test_guest_cannot_read_foreign_work_item(self, guest_client, workspace, project, foreign_issue):
+        url = WORK_ITEM_BY_IDENTIFIER_URL.format(
+            slug=workspace.slug,
+            project_identifier=project.identifier,
+            sequence_id=foreign_issue.sequence_id,
+        )
+        response = guest_client.get(url)
+
+        assert response.status_code == status.HTTP_403_FORBIDDEN, (
+            f"Restricted guest resolved a foreign work item: {response.status_code} {getattr(response, 'data', None)!r}"
+        )
+
+    @pytest.mark.django_db
+    def test_guest_can_read_their_own_work_item(self, guest_client, workspace, project, own_issue):
+        """Positive control: the guard must not lock a guest out of their own item."""
+        url = WORK_ITEM_BY_IDENTIFIER_URL.format(
+            slug=workspace.slug,
+            project_identifier=project.identifier,
+            sequence_id=own_issue.sequence_id,
+        )
+        response = guest_client.get(url)
+
+        assert response.status_code == status.HTTP_200_OK, (
+            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
+        )
+        assert str(response.data["id"]) == str(own_issue.id)
+
+    @pytest.mark.django_db
+    def test_full_member_can_read_any_work_item(self, session_client, workspace, project, foreign_issue):
+        """Positive control: a full member is unaffected."""
+        url = WORK_ITEM_BY_IDENTIFIER_URL.format(
+            slug=workspace.slug,
+            project_identifier=project.identifier,
+            sequence_id=foreign_issue.sequence_id,
+        )
+        response = session_client.get(url)
+
+        assert response.status_code == status.HTTP_200_OK, (
+            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
+        )
+        assert str(response.data["id"]) == str(foreign_issue.id)
+
+    @pytest.mark.django_db
+    def test_guest_with_view_all_features_can_read_any_work_item(self, guest_client, workspace, project, foreign_issue):
+        """Positive control: the guard is conditional on the project setting."""
+        project.guest_view_all_features = True
+        project.save(update_fields=["guest_view_all_features"])
+
+        url = WORK_ITEM_BY_IDENTIFIER_URL.format(
+            slug=workspace.slug,
+            project_identifier=project.identifier,
+            sequence_id=foreign_issue.sequence_id,
+        )
+        response = guest_client.get(url)
+
+        assert response.status_code == status.HTTP_200_OK, (
+            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
+        )
+
+    @pytest.mark.django_db
+    def test_non_member_is_refused(self, workspace, project, foreign_issue):
+        """A user with no project membership must still be refused outright."""
+        outsider = _make_user("outsider")
+        WorkspaceMember.objects.create(workspace=workspace, member=outsider, role=15)
+
+        url = WORK_ITEM_BY_IDENTIFIER_URL.format(
+            slug=workspace.slug,
+            project_identifier=project.identifier,
+            sequence_id=foreign_issue.sequence_id,
+        )
+        response = _client_for(outsider).get(url)
+
+        assert response.status_code == status.HTTP_403_FORBIDDEN, (
+            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
+        )
+
+
+@pytest.fixture
+def workspace_view(db, workspace, create_user):
+    """A global (project-less) workspace view owned by the workspace owner."""
+    view = IssueView(name="Global view", workspace=workspace, owned_by=create_user, access=1)
+    view.save()
+    return view
+
+
+@pytest.mark.contract
+class TestWorkspaceViewRetrieveRequiresMembership:
+    """Reading a global view must require membership of its workspace."""
+
+    @pytest.mark.django_db
+    def test_outsider_cannot_read_a_global_view(self, workspace, workspace_view):
+        """A user in a different workspace entirely, holding only the view id."""
+        outsider = _make_user("outsider")
+        other_workspace = Workspace.objects.create(
+            name="Outsider Workspace",
+            owner=outsider,
+            slug=f"outsider-{uuid4().hex[:8]}",
+        )
+        WorkspaceMember.objects.create(workspace=other_workspace, member=outsider, role=20)
+
+        url = WORKSPACE_VIEW_DETAIL_URL.format(slug=workspace.slug, pk=workspace_view.id)
+        response = _client_for(outsider).get(url)
+
+        assert response.status_code in (
+            status.HTTP_403_FORBIDDEN,
+            status.HTTP_404_NOT_FOUND,
+        ), f"Non-member read a global view: {response.status_code} {getattr(response, 'data', None)!r}"
+
+    @pytest.mark.django_db
+    def test_workspace_member_can_read_a_global_view(self, session_client, workspace, workspace_view):
+        """Positive control: a member of the workspace still reads it."""
+        url = WORKSPACE_VIEW_DETAIL_URL.format(slug=workspace.slug, pk=workspace_view.id)
+        response = session_client.get(url)
+
+        assert response.status_code == status.HTTP_200_OK, (
+            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
+        )
+        assert str(response.data["id"]) == str(workspace_view.id)
+
+    @pytest.mark.django_db
+    def test_workspace_guest_can_read_a_global_view(self, workspace, workspace_view):
+        """Positive control: the role set matches list(), which permits guests."""
+        guest_user = _make_user("wsguest")
+        WorkspaceMember.objects.create(workspace=workspace, member=guest_user, role=5)
+
+        url = WORKSPACE_VIEW_DETAIL_URL.format(slug=workspace.slug, pk=workspace_view.id)
+        response = _client_for(guest_user).get(url)
+
+        assert response.status_code == status.HTTP_200_OK, (
+            f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
+        )