mirror of
https://github.com/makeplane/plane.git
synced 2026-09-02 12:09:14 +02:00
[WEB-8353] fix(security): extend guest issue visibility guard to comments + reactions
The attachment and activity endpoints already route restricted guests through issue_hidden_from_guest, but this PR's own docstring names comments as in scope too — that part was never wired up. A restricted guest blocked from GET .../history/ on a hidden issue could still call GET .../comments/ (list and retrieve) and read all comment content on that same issue, and could create/destroy CommentReaction rows keyed by comment_id with no issue-visibility check at all. Apply the same guard to IssueCommentViewSet.list/.retrieve. For CommentReactionViewSet.create/.destroy, which only receive comment_id (not issue_id) from the URL, resolve the parent issue first and then apply the identical check. Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
@@ -18,7 +18,7 @@ from rest_framework import status
|
||||
# Module imports
|
||||
from .. import BaseViewSet
|
||||
from plane.app.serializers import IssueCommentSerializer, CommentReactionSerializer
|
||||
from plane.app.permissions import allow_permission, ROLE
|
||||
from plane.app.permissions import allow_permission, issue_hidden_from_guest, ROLE
|
||||
from plane.db.models import IssueComment, ProjectMember, CommentReaction, Project, Issue
|
||||
from plane.bgtasks.issue_activities_task import issue_activity
|
||||
from plane.utils.host import base_host
|
||||
@@ -60,6 +60,28 @@ class IssueCommentViewSet(BaseViewSet):
|
||||
.distinct()
|
||||
)
|
||||
|
||||
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
|
||||
def list(self, request, slug, project_id, issue_id):
|
||||
# A restricted guest may only see the comments of issues they created,
|
||||
# mirroring the issue-detail visibility rule.
|
||||
if issue_hidden_from_guest(request, slug, project_id, issue_id):
|
||||
return Response(
|
||||
{"error": "You are not allowed to view this issue"},
|
||||
status=status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
return super().list(request, slug, project_id, issue_id)
|
||||
|
||||
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
|
||||
def retrieve(self, request, slug, project_id, issue_id, pk):
|
||||
# A restricted guest may only see the comments of issues they created,
|
||||
# mirroring the issue-detail visibility rule.
|
||||
if issue_hidden_from_guest(request, slug, project_id, issue_id):
|
||||
return Response(
|
||||
{"error": "You are not allowed to view this issue"},
|
||||
status=status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
return super().retrieve(request, slug, project_id, issue_id, pk)
|
||||
|
||||
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
|
||||
def create(self, request, slug, project_id, issue_id):
|
||||
project = Project.objects.get(pk=project_id)
|
||||
@@ -182,6 +204,19 @@ class CommentReactionViewSet(BaseViewSet):
|
||||
|
||||
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
|
||||
def create(self, request, slug, project_id, comment_id):
|
||||
# A restricted guest may only react to comments on issues they created,
|
||||
# mirroring the issue-detail visibility rule. Comment reactions are
|
||||
# keyed by comment_id, so resolve the parent issue first.
|
||||
issue_id = (
|
||||
IssueComment.objects.filter(pk=comment_id, workspace__slug=slug, project_id=project_id)
|
||||
.values_list("issue_id", flat=True)
|
||||
.first()
|
||||
)
|
||||
if issue_id and issue_hidden_from_guest(request, slug, project_id, issue_id):
|
||||
return Response(
|
||||
{"error": "You are not allowed to view this issue"},
|
||||
status=status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
try:
|
||||
serializer = CommentReactionSerializer(data=request.data)
|
||||
if serializer.is_valid():
|
||||
@@ -211,6 +246,19 @@ class CommentReactionViewSet(BaseViewSet):
|
||||
|
||||
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
|
||||
def destroy(self, request, slug, project_id, comment_id, reaction_code):
|
||||
# A restricted guest may only react to comments on issues they created,
|
||||
# mirroring the issue-detail visibility rule. Comment reactions are
|
||||
# keyed by comment_id, so resolve the parent issue first.
|
||||
issue_id = (
|
||||
IssueComment.objects.filter(pk=comment_id, workspace__slug=slug, project_id=project_id)
|
||||
.values_list("issue_id", flat=True)
|
||||
.first()
|
||||
)
|
||||
if issue_id and issue_hidden_from_guest(request, slug, project_id, issue_id):
|
||||
return Response(
|
||||
{"error": "You are not allowed to view this issue"},
|
||||
status=status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
comment_reaction = CommentReaction.objects.get(
|
||||
workspace__slug=slug,
|
||||
project_id=project_id,
|
||||
|
||||
@@ -12,6 +12,8 @@ not replicate that restriction:
|
||||
* ``IssueAttachmentEndpoint.get`` (v1 ``.../issue-attachments/``)
|
||||
* ``IssueAttachmentV2Endpoint.get`` (v2 ``/assets/v2/.../attachments/``)
|
||||
* ``IssueActivityEndpoint.get`` (``.../history/``)
|
||||
* ``IssueCommentViewSet.list``/``.retrieve`` (``.../comments/``)
|
||||
* ``CommentReactionViewSet.create``/``.destroy`` (``.../comments/<id>/reactions/``)
|
||||
|
||||
Each is decorated ``@allow_permission([ADMIN, MEMBER, GUEST])`` and queried by
|
||||
``issue_id`` only, so a restricted guest who is a legitimate project member could
|
||||
@@ -33,6 +35,7 @@ from rest_framework.test import APIClient
|
||||
from plane.db.models import (
|
||||
FileAsset,
|
||||
Issue,
|
||||
IssueComment,
|
||||
Project,
|
||||
ProjectMember,
|
||||
User,
|
||||
@@ -42,6 +45,10 @@ from plane.db.models import (
|
||||
V1_ATTACH_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/issue-attachments/"
|
||||
V2_ATTACH_URL = "/api/assets/v2/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/attachments/"
|
||||
HISTORY_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/history/"
|
||||
COMMENT_LIST_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/comments/"
|
||||
COMMENT_DETAIL_URL = "/api/workspaces/{slug}/projects/{project_id}/issues/{issue_id}/comments/{pk}/"
|
||||
REACTION_URL = "/api/workspaces/{slug}/projects/{project_id}/comments/{comment_id}/reactions/"
|
||||
REACTION_DETAIL_URL = "/api/workspaces/{slug}/projects/{project_id}/comments/{comment_id}/reactions/{reaction_code}/"
|
||||
|
||||
|
||||
def _make_issue(name, project, workspace, author):
|
||||
@@ -52,6 +59,12 @@ def _make_issue(name, project, workspace, author):
|
||||
return issue
|
||||
|
||||
|
||||
def _make_comment(issue, project, workspace, actor):
|
||||
return IssueComment.objects.create(
|
||||
issue=issue, project=project, actor=actor, comment_html="<p>secret comment</p>"
|
||||
)
|
||||
|
||||
|
||||
def _make_attachment(issue, project, workspace):
|
||||
return FileAsset.objects.create(
|
||||
attributes={"name": "secret.pdf", "type": "application/pdf", "size": 100},
|
||||
@@ -114,6 +127,18 @@ def foreign_issue(db, workspace, project, create_user):
|
||||
return issue
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def own_issue_comment(db, workspace, project, own_issue, guest):
|
||||
"""A comment on the guest's own issue."""
|
||||
return _make_comment(own_issue, project, workspace, guest)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def foreign_issue_comment(db, workspace, project, foreign_issue, create_user):
|
||||
"""A comment on an issue authored by someone other than the guest."""
|
||||
return _make_comment(foreign_issue, project, workspace, create_user)
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
@pytest.mark.django_db
|
||||
class TestGuestIssueSubresourceScope:
|
||||
@@ -167,6 +192,56 @@ class TestGuestIssueSubresourceScope:
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
def test_guest_blocked_comments_list(self, guest_client, workspace, project, foreign_issue, foreign_issue_comment):
|
||||
response = guest_client.get(
|
||||
COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=foreign_issue.id)
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
def test_guest_blocked_comment_retrieve(
|
||||
self, guest_client, workspace, project, foreign_issue, foreign_issue_comment
|
||||
):
|
||||
response = guest_client.get(
|
||||
COMMENT_DETAIL_URL.format(
|
||||
slug=workspace.slug,
|
||||
project_id=project.id,
|
||||
issue_id=foreign_issue.id,
|
||||
pk=foreign_issue_comment.id,
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
def test_guest_blocked_comment_reaction_create(
|
||||
self, guest_client, workspace, project, foreign_issue, foreign_issue_comment
|
||||
):
|
||||
response = guest_client.post(
|
||||
REACTION_URL.format(slug=workspace.slug, project_id=project.id, comment_id=foreign_issue_comment.id),
|
||||
{"reaction": "like"},
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
def test_guest_blocked_comment_reaction_destroy(
|
||||
self, guest_client, workspace, project, foreign_issue, foreign_issue_comment
|
||||
):
|
||||
response = guest_client.delete(
|
||||
REACTION_DETAIL_URL.format(
|
||||
slug=workspace.slug,
|
||||
project_id=project.id,
|
||||
comment_id=foreign_issue_comment.id,
|
||||
reaction_code="like",
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
# ---- the guest still sees their OWN issue's sub-resources ----------------
|
||||
|
||||
def test_guest_allowed_own_issue_attachments(self, guest_client, workspace, project, own_issue):
|
||||
@@ -178,8 +253,66 @@ class TestGuestIssueSubresourceScope:
|
||||
)
|
||||
assert len(response.data) == 1, f"Guest should see their own attachment: {response.data!r}"
|
||||
|
||||
def test_guest_allowed_own_issue_comments_list(
|
||||
self, guest_client, workspace, project, own_issue, own_issue_comment
|
||||
):
|
||||
response = guest_client.get(
|
||||
COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=own_issue.id)
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
assert len(response.data) == 1, f"Guest should see their own comment: {response.data!r}"
|
||||
|
||||
def test_guest_allowed_own_issue_comment_retrieve(
|
||||
self, guest_client, workspace, project, own_issue, own_issue_comment
|
||||
):
|
||||
response = guest_client.get(
|
||||
COMMENT_DETAIL_URL.format(
|
||||
slug=workspace.slug, project_id=project.id, issue_id=own_issue.id, pk=own_issue_comment.id
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
def test_guest_allowed_own_issue_comment_reaction_create(
|
||||
self, guest_client, workspace, project, own_issue, own_issue_comment
|
||||
):
|
||||
response = guest_client.post(
|
||||
REACTION_URL.format(slug=workspace.slug, project_id=project.id, comment_id=own_issue_comment.id),
|
||||
{"reaction": "like"},
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == status.HTTP_201_CREATED, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
# ---- positive controls: members and unrestricted guests unaffected -------
|
||||
|
||||
def test_member_reads_foreign_issue_comments(
|
||||
self, session_client, workspace, project, foreign_issue, foreign_issue_comment
|
||||
):
|
||||
"""A full project member still reads any issue's comments."""
|
||||
response = session_client.get(
|
||||
COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=foreign_issue.id)
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert len(response.data) == 1
|
||||
|
||||
def test_unrestricted_guest_reads_foreign_issue_comments(
|
||||
self, guest_client, workspace, project, foreign_issue, foreign_issue_comment
|
||||
):
|
||||
"""When guest_view_all_features is enabled, the guest sees all comments."""
|
||||
project.guest_view_all_features = True
|
||||
project.save(update_fields=["guest_view_all_features"])
|
||||
|
||||
response = guest_client.get(
|
||||
COMMENT_LIST_URL.format(slug=workspace.slug, project_id=project.id, issue_id=foreign_issue.id)
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert len(response.data) == 1
|
||||
|
||||
def test_member_reads_foreign_issue_attachments(self, session_client, workspace, project, foreign_issue):
|
||||
"""A full project member still reads any issue's attachments."""
|
||||
response = session_client.get(
|
||||
|
||||
Reference in New Issue
Block a user