mirror of
https://github.com/makeplane/plane.git
synced 2026-09-02 03:59:00 +02:00
[INFRA-499] fix(security): reapply guest ownership restriction in WorkspaceViewViewSet.retrieve
list() already restricts a GUEST-role member to global views they own, but retrieve() only checked workspace membership, not ownership. Since get_queryset()'s access-based clause is unconditionally true (access is read-only and defaults to Public), a guest who owned no views could still fetch any other member's global view directly by id. Mirror the project-scoped IssueViewViewSet.retrieve(), which already reapplies this same restriction, and update the PR's own contract test (test_workspace_guest_can_read_a_global_view) which had asserted 200 for this exact case — it now asserts 404, matching the "missing view" branch already established on this endpoint. Added a positive control confirming a guest can still read a view they own. Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
@@ -141,6 +141,22 @@ class WorkspaceViewViewSet(BaseViewSet):
|
||||
status=status.HTTP_404_NOT_FOUND,
|
||||
)
|
||||
|
||||
# Reapply the same guest-ownership restriction list() enforces above.
|
||||
# get_queryset()'s `Q(owned_by=request.user) | Q(access=1)` is vacuous
|
||||
# (see comment above), so without this a GUEST who owns no views could
|
||||
# still fetch any other member's global view directly by id. Answer 404
|
||||
# rather than 403, consistent with the "missing view" branch above —
|
||||
# this endpoint does not otherwise distinguish "not found" from "not
|
||||
# visible to you".
|
||||
if (
|
||||
WorkspaceMember.objects.filter(workspace__slug=slug, member=request.user, role=5, is_active=True).exists()
|
||||
and issue_view.owned_by_id != request.user.id
|
||||
):
|
||||
return Response(
|
||||
{"error": "The required object does not exist."},
|
||||
status=status.HTTP_404_NOT_FOUND,
|
||||
)
|
||||
|
||||
serializer = IssueViewSerializer(issue_view)
|
||||
recent_visited_task.delay(
|
||||
slug=slug,
|
||||
|
||||
@@ -17,7 +17,8 @@ directions — one of the two turned out to be guarded after all.
|
||||
``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``.
|
||||
membership, matching ``list`` — and, like ``list``, restricts a GUEST to
|
||||
views they own, since guests pass the membership check too.
|
||||
|
||||
2. **Regression coverage only, no fix.** ``IssueDetailIdentifierEndpoint``
|
||||
(``/workspaces/<slug>/work-items/<PROJ>-<n>/``) resolves a work item by its
|
||||
@@ -264,14 +265,35 @@ class TestWorkspaceViewRetrieveRequiresMembership:
|
||||
)
|
||||
|
||||
@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."""
|
||||
def test_workspace_guest_cannot_read_a_global_view_they_do_not_own(self, workspace, workspace_view):
|
||||
"""The role set matches list(), which permits guests as members — but list()
|
||||
also restricts a GUEST to views they own. retrieve() must reapply that
|
||||
same restriction: this guest owns no views, so a view owned by someone
|
||||
else must not be readable by id even though guests may pass the
|
||||
workspace-membership check above."""
|
||||
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_404_NOT_FOUND, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_workspace_guest_can_read_a_global_view_they_own(self, workspace):
|
||||
"""Positive control: a guest reading a view they own themselves must
|
||||
still succeed — the fix must not over-restrict."""
|
||||
guest_user = _make_user("wsguest-owner")
|
||||
WorkspaceMember.objects.create(workspace=workspace, member=guest_user, role=5)
|
||||
own_view = IssueView(name="Guest's own view", workspace=workspace, owned_by=guest_user, access=1)
|
||||
own_view.save()
|
||||
|
||||
url = WORKSPACE_VIEW_DETAIL_URL.format(slug=workspace.slug, pk=own_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}"
|
||||
)
|
||||
assert str(response.data["id"]) == str(own_view.id)
|
||||
|
||||
Reference in New Issue
Block a user