From dc9d80b2d2a499b967f0b541e083b283f463719f Mon Sep 17 00:00:00 2001 From: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:37:39 +0530 Subject: [PATCH] [WEB-8060] fix(security): enforce authz on is_active member (de)activation (#9367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [WEB-8060] fix(security): enforce authz on is_active member (de)activation ProjectMemberViewSet.partial_update nested every authorization guard inside `if "role" in request.data:`. Because ProjectMemberSerializer exposes is_active through fields="__all__" with no read-only, a project GUEST could PATCH {"is_active": false} while omitting "role" to deactivate any member — including admins — and take over the project (GHSA-hpgm-9r34-c4x5 / GHSA-25gg-cxm8-g7h9). Add an independent is_active guard, mirroring the role block and destroy(): only a project admin (or workspace admin) may (de)activate a member, and never one whose role is equal to or higher than the requester's own. Adds contract regression tests covering guest/member deactivation attempts and the legitimate project-admin path. Co-authored-by: Plane AI * [WEB-8060] test(security): cover workspace-admin is_active bypass Address Copilot review on #9367: add a positive-control test asserting a workspace admin holding only a project GUEST role can still deactivate a project admin (the intended is_workspace_admin bypass), so future changes cannot silently remove it. Co-authored-by: Plane AI --------- Co-authored-by: Plane AI --- apps/api/plane/app/views/project/member.py | 19 ++ .../test_project_member_is_active_authz.py | 172 ++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py diff --git a/apps/api/plane/app/views/project/member.py b/apps/api/plane/app/views/project/member.py index e747f573e7..973462182a 100644 --- a/apps/api/plane/app/views/project/member.py +++ b/apps/api/plane/app/views/project/member.py @@ -261,6 +261,25 @@ class ProjectMemberViewSet(BaseViewSet): status=status.HTTP_400_BAD_REQUEST, ) + # Guard privileged `is_active` mutations (member (de)activation). These are NOT + # covered by the role block above, so without this check a GUEST could PATCH + # {"is_active": false} while omitting "role" to deactivate any member — including + # admins — and take over the project. Mirror the role block and destroy(): only a + # project admin (or workspace admin) may (de)activate a member, and never one whose + # role is equal to or higher than the requester's own. + if "is_active" in request.data: + if requested_project_member.role < ROLE.ADMIN.value and not is_workspace_admin: + return Response( + {"error": "You do not have permission to update member status"}, + status=status.HTTP_403_FORBIDDEN, + ) + + if project_member.role >= requested_project_member.role and not is_workspace_admin: + return Response( + {"error": "You cannot update the status of a member with a role equal to or higher than your own"}, + status=status.HTTP_403_FORBIDDEN, + ) + serializer = ProjectMemberSerializer(project_member, data=request.data, partial=True) if serializer.is_valid(): diff --git a/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py b/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py new file mode 100644 index 0000000000..d59cad0591 --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_project_member_is_active_authz.py @@ -0,0 +1,172 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +""" +Regression tests for GHSA-hpgm-9r34-c4x5 / GHSA-25gg-cxm8-g7h9. + +A project GUEST (or MEMBER) must not be able to (de)activate other project +members by PATCHing ``{"is_active": false}`` while omitting the ``role`` field. +Before the fix, every authorization guard in ``ProjectMemberViewSet.partial_update`` +lived inside ``if "role" in request.data:`` and ``is_active`` was writable through +``ProjectMemberSerializer(fields="__all__")`` — so a guest could deactivate any +member, including admins, and take over the project. +""" + +import uuid + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from plane.db.models import ( + Project, + ProjectMember, + User, + WorkspaceMember, +) + + +def _member_detail_url(slug: str, project_id: uuid.UUID, pk: uuid.UUID) -> str: + return f"/api/workspaces/{slug}/projects/{project_id}/members/{pk}/" + + +def _make_user(email: str) -> User: + local_part = email.split("@")[0] + user = User.objects.create(email=email, username=local_part, first_name=local_part) + user.set_password("test-password") + user.save() + return user + + +def _add_member(workspace, project, user, *, ws_role: int, project_role: int) -> ProjectMember: + WorkspaceMember.objects.create(workspace=workspace, member=user, role=ws_role, is_active=True) + return ProjectMember.objects.create( + workspace=workspace, project=project, member=user, role=project_role, is_active=True + ) + + +@pytest.fixture +def project(db, workspace, create_user): + """A project owned by ``create_user`` (workspace owner / admin).""" + project = Project.objects.create( + name="Secure Project", + identifier="SEC", + workspace=workspace, + created_by=create_user, + ) + # create_user is the workspace owner (role=20 via the workspace fixture); + # make them a project ADMIN too — this is the takeover victim. + ProjectMember.objects.create( + workspace=workspace, project=project, member=create_user, role=20, is_active=True + ) + return project + + +@pytest.mark.contract +@pytest.mark.django_db +class TestProjectMemberIsActiveAuthz: + def test_guest_cannot_deactivate_admin(self, workspace, project, create_user): + """A project GUEST must not deactivate a project ADMIN via is_active.""" + attacker = _make_user("guest-attacker@plane.so") + # non-workspace-admin (role 15) so is_workspace_admin bypass does not apply, + # project GUEST (role 5) + _add_member(workspace, project, attacker, ws_role=15, project_role=5) + victim = ProjectMember.objects.get(project=project, member=create_user) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(workspace.slug, project.id, victim.id), + {"is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + victim.refresh_from_db() + assert victim.is_active is True + + def test_member_cannot_deactivate_admin(self, workspace, project, create_user): + """A project MEMBER must not deactivate a project ADMIN via is_active.""" + attacker = _make_user("member-attacker@plane.so") + _add_member(workspace, project, attacker, ws_role=15, project_role=15) + victim = ProjectMember.objects.get(project=project, member=create_user) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(workspace.slug, project.id, victim.id), + {"is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + victim.refresh_from_db() + assert victim.is_active is True + + def test_guest_cannot_deactivate_peer_guest(self, workspace, project): + """A GUEST cannot deactivate another GUEST either (role check applies to all).""" + attacker = _make_user("guest-a@plane.so") + peer = _make_user("guest-b@plane.so") + _add_member(workspace, project, attacker, ws_role=15, project_role=5) + peer_member = _add_member(workspace, project, peer, ws_role=15, project_role=5) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(workspace.slug, project.id, peer_member.id), + {"is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + peer_member.refresh_from_db() + assert peer_member.is_active is True + + def test_project_admin_can_deactivate_member(self, workspace, project): + """Positive control: a project ADMIN (non-workspace-admin) may deactivate a MEMBER.""" + admin = _make_user("project-admin@plane.so") + target = _make_user("plain-member@plane.so") + # admin is a workspace MEMBER (15) but project ADMIN (20) — exercises the + # role-comparison guard rather than the workspace-admin bypass. + _add_member(workspace, project, admin, ws_role=15, project_role=20) + target_member = _add_member(workspace, project, target, ws_role=15, project_role=15) + + client = APIClient() + client.force_authenticate(user=admin) + response = client.patch( + _member_detail_url(workspace.slug, project.id, target_member.id), + {"is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + target_member.refresh_from_db() + assert target_member.is_active is False + + def test_workspace_admin_with_low_project_role_can_deactivate(self, workspace, project, create_user): + """ + Positive control: the intended workspace-admin bypass is preserved. + + A workspace ADMIN (role 20) may deactivate any project member — even a + project ADMIN — despite holding only a project GUEST role, because + is_workspace_admin short-circuits the role-comparison guard. Locks in the + bypass so future changes don't silently remove it. + """ + ws_admin = _make_user("ws-admin@plane.so") + # workspace ADMIN (20) but only a project GUEST (5) + _add_member(workspace, project, ws_admin, ws_role=20, project_role=5) + # victim is the project ADMIN (create_user) set up by the `project` fixture + victim = ProjectMember.objects.get(project=project, member=create_user) + + client = APIClient() + client.force_authenticate(user=ws_admin) + response = client.patch( + _member_detail_url(workspace.slug, project.id, victim.id), + {"is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + victim.refresh_from_db() + assert victim.is_active is False