feat: protect volunteer api

app/api/deps.py

@@ -1,10 +1,11 @@
 import time
 from collections.abc import Generator
+from uuid import UUID
 
 import jwt
 from fastapi import Depends, HTTPException, Path, status
 from fastapi.security import OAuth2PasswordBearer
-from sqlalchemy import UUID, exists, select
+from sqlalchemy import exists, select
 from sqlalchemy.orm import Session
 
 from app.api.utils import get_project_organization_id
@@ -65,7 +66,7 @@ def require_super_admin(current_user: User = Depends(get_current_user)) -> User:
 
 
 def _has_org_role(
-    session: Session, user_id: UUID, project_id: str, *allowed_roles: OrgRole
+    session: Session, user_id: UUID, project_id: UUID, *allowed_roles: OrgRole
 ) -> bool:
     """Single query: does user_id hold one of allowed_roles in the
     organization that owns project_id? Replaces loading the user's full
@@ -97,7 +98,7 @@ def _is_commission_member(session: Session, user_id: str, commission_id: str | N
 
 def require_org_role(*allowed_roles: OrgRole):
     def dependency(
-        project_id: str = Path(...),
+        project_id: UUID = Path(...),
         session: Session = Depends(get_session),
         current_user: User = Depends(get_current_user),
     ) -> User:
@@ -122,7 +123,7 @@ def require_commission_membership():
 
 
 def assert_commission_ownership(
-    session: Session, current_user: User, project_id: str, commission_id: str | None
+    session: Session, current_user: User, project_id: UUID, commission_id: str | None
 ) -> None:
     """Called inside create/update handlers once the payload's commission_id
     is known. org_admin/respo_benevole: any commission is fine. respo_commission:
@@ -143,14 +144,18 @@ def require_organization_role(*allowed_roles: OrgRole):
     project. Super admins always pass."""
 
     def dependency(
-        organization_id: str = Path(...),
+        organization_id: UUID = Path(...),
         current_user: User = Depends(get_current_user),
     ) -> User:
         if current_user.global_role == GlobalRole.SUPER_ADMIN:
             return current_user
 
         role = next(
-            (m.role for m in current_user.organizations if m.organization_id == organization_id),
+            (
+                m.role
+                for m in current_user.organizations
+                if m.organization_id == str(organization_id)
+            ),
             None,
         )
         if role is None or role not in allowed_roles:

app/api/endpoints/organizations.py

@@ -6,11 +6,11 @@ from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import Session, selectinload
 
 from app.api import deps
-from app.models import GlobalRole, Organization, User, UserOrganization, OrgRole
+from app.models import GlobalRole, Organization, OrgRole, User, UserOrganization
 from app.schemas.requests import (
-    OrganizationRequest,
     OrganizationMembershipCreateRequest,
     OrganizationMembershipUpdateRequest,
+    OrganizationRequest,
 )
 from app.schemas.responses import (
     OrganizationDetailResponse,
@@ -62,11 +62,15 @@ async def get_organization(
 ):
     """Get organization detail including members.
     super_admin, or any member of the organization, may view it."""
-    org = session.execute(
-        select(Organization)
-        .where(Organization.id == str(organization_id))
-        .options(selectinload(Organization.memberships).selectinload(UserOrganization.user))
-    ).scalars().first()
+    org = (
+        session.execute(
+            select(Organization)
+            .where(Organization.id == str(organization_id))
+            .options(selectinload(Organization.memberships).selectinload(UserOrganization.user))
+        )
+        .scalars()
+        .first()
+    )
     if org is None:
         raise HTTPException(404, "Organization not found")
 
@@ -115,6 +119,7 @@ async def delete_organization(
 
 # ---- Membership management ----
 
+
 @router.post("/{organization_id}/members", response_model=OrganizationMembershipResponse)
 async def add_member(
     organization_id: UUID,
@@ -138,16 +143,16 @@ async def add_member(
     if existing is not None:
         raise HTTPException(400, "User is already a member of this organization")
 
-    membership = UserOrganization(user_id=payload.user_id, organization_id=organization_id, role=payload.role)
+    membership = UserOrganization(
+        user_id=payload.user_id, organization_id=organization_id, role=payload.role
+    )
     session.add(membership)
     session.commit()
     session.refresh(membership)
     return membership
 
 
-@router.patch(
-    "/{organization_id}/members/{user_id}", response_model=OrganizationMembershipResponse
-)
+@router.patch("/{organization_id}/members/{user_id}", response_model=OrganizationMembershipResponse)
 async def update_member_role(
     organization_id: UUID,
     user_id: UUID,
@@ -177,4 +182,4 @@ async def remove_member(
     if membership is None:
         raise HTTPException(404, "Membership not found")
     session.delete(membership)
-    session.commit()
+    session.commit()

app/api/endpoints/volunteers.py

@@ -5,22 +5,26 @@ from sqlalchemy import delete, select
 from sqlalchemy.orm import Session
 
 from app.api import deps
-from app.api.utils import assert_project_exists_or_404, update_object_from_payload, verify_id_list
-from app.models import Slot, User, Volunteer, association_table_volunteer_slot
+from app.api.utils import update_object_from_payload, verify_id_list
+from app.models import OrgRole, Slot, User, Volunteer, association_table_volunteer_slot
 from app.schemas.requests import VolunteerCreateRequest, VolunteerUpdateRequest
 from app.schemas.responses import VolunteerResponse
 
 router = APIRouter(prefix="/project/{project_id}", tags=["volunteers"])
 
+READ_ROLES = (OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION)
+WRITE_ROLES = (OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE)
+
 
 @router.get("/volunteers", response_model=list[VolunteerResponse])
 async def list_project_volunteers(
     project_id: UUID,
-    current_user: User = Depends(deps.get_current_user),
+    current_user: User = Depends(deps.require_org_role(*READ_ROLES)),
     session: Session = Depends(deps.get_session),
 ):
-    """List volunteers from project"""
-    assert_project_exists_or_404(session, project_id)
+    """List volunteers from project.
+    respo_commission has read-only access -- they need to see who is
+    assigned to work with them, but do not manage volunteer records."""
     results = session.execute(select(Volunteer).where(Volunteer.project_id == project_id))
     return results.scalars().all()
 
@@ -29,14 +33,12 @@ async def list_project_volunteers(
 async def create_volunteer(
     project_id: UUID,
     new_volunteer: VolunteerCreateRequest,
-    current_user: User = Depends(deps.get_current_user),
+    current_user: User = Depends(deps.require_org_role(*WRITE_ROLES)),
     session: Session = Depends(deps.get_session),
 ):
     """Create a new volunteer to the project"""
-    assert_project_exists_or_404(session, project_id)
     input_dict = new_volunteer.model_dump()
 
-    # Extract slots list from input dict
     slots: list[UUID] = []
     if input_dict["slots"] is not None:
         slots = input_dict["slots"]
@@ -45,7 +47,6 @@ async def create_volunteer(
 
     volunteer = Volunteer(project_id=project_id, **input_dict)
     session.add(volunteer)
-    # commit to optain an id for the volunteer
     session.commit()
 
     if len(slots) > 0:
@@ -55,6 +56,7 @@ async def create_volunteer(
             )
         )
     session.commit()
+    session.refresh(volunteer)
     return volunteer
 
 
@@ -63,7 +65,7 @@ async def update_volunteer(
     project_id: UUID,
     volunteer_id: UUID,
     new_volunteer: VolunteerUpdateRequest,
-    current_user: User = Depends(deps.get_current_user),
+    current_user: User = Depends(deps.require_org_role(*WRITE_ROLES)),
     session: Session = Depends(deps.get_session),
 ):
     """Update a volunteer from the project"""
@@ -72,17 +74,14 @@ async def update_volunteer(
         raise HTTPException(status_code=404, detail="Volunteer not found")
 
     input_dict = new_volunteer.model_dump(exclude_unset=True)
-    # Extract slots list from input dict
     if "slots" in input_dict:
         slots: list[UUID] = input_dict["slots"]
         await verify_id_list(session, slots, project_id, Slot, "Invalid slot list")
-        # Remove previous values
         session.execute(
             association_table_volunteer_slot.delete().where(
                 association_table_volunteer_slot.c.volunteer_id == volunteer.id
             )
         )
-        # Add the new slots
         if len(slots) > 0:
             session.execute(
                 association_table_volunteer_slot.insert().values(
@@ -101,7 +100,7 @@ async def update_volunteer(
 async def delete_volunteer(
     project_id: UUID,
     volunteer_id: UUID,
-    current_user: User = Depends(deps.get_current_user),
+    current_user: User = Depends(deps.require_org_role(*WRITE_ROLES)),
     session: Session = Depends(deps.get_session),
 ):
     """Delete a volunteer from the project"""

app/tests/test_volunteer.py

@@ -6,21 +6,16 @@ from sqlalchemy import select
 from sqlalchemy.orm import Session
 
 from app.main import app
-from app.models import Project, Slot, Sms, Volunteer
-from app.tests.conftest import (
-    default_project_id,
-    default_slot_id,
-    default_volunteer_id,
-)
+from app.models import OrgRole, Project, Slot, Sms, Volunteer
+from app.tests.conftest import default_project_id, default_slot_id, default_volunteer_id
+
+pytestmark = pytest.mark.asyncio
+
+WRITE_FORBIDDEN_ROLES = [OrgRole.RESPO_COMMISSION, None]
 
 
 class TestListVolunteer:
-    async def test_read_list_not_authenticated(
-        self,
-        client: AsyncClient,
-        default_user_headers: dict,
-        default_public_project: Project,
-    ):
+    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
         response = await client.get(
             app.url_path_for("list_project_volunteers", project_id=default_project_id),
         )
@@ -33,87 +28,106 @@ class TestListVolunteer:
     async def test_read_list_fails(
         self,
         client: AsyncClient,
-        default_user_headers: dict,
         default_public_project: Project,
         make_org_user,
         project_id,
         status_code,
     ):
-        _, headers = make_org_user()
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
         response = await client.get(
             app.url_path_for("list_project_volunteers", project_id=project_id),
             headers=headers,
         )
         assert response.status_code == status_code
 
-    async def test_real_project(
-        self, client: AsyncClient, default_user_headers: dict, default_public_project: Project
+    @pytest.mark.parametrize(
+        "role", [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE, OrgRole.RESPO_COMMISSION]
+    )
+    async def test_readable_by_all_org_roles(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        role,
     ):
+        """respo_commission can read volunteers -- needs visibility into who
+        they'll be working with -- even though they can't manage them."""
+        _, headers = make_org_user(role=role)
         response = await client.get(
             app.url_path_for("list_project_volunteers", project_id=default_project_id),
-            headers=default_user_headers,
+            headers=headers,
         )
         assert response.status_code == 200
         data = response.json()
         assert len(data) == 1
-        volunteer_response = data[0]
-        assert volunteer_response["name"] == "Arthur"
-        assert volunteer_response["id"] == default_volunteer_id
-        assert "created_at" in volunteer_response
-
+        assert data[0]["name"] == "Arthur"
+        assert data[0]["id"] == default_volunteer_id
+        assert "created_at" in data[0]
 
-class TestCreateVolunteer:
-    async def test_create_volunteer_without_auth(
+    async def test_no_membership_forbidden(
         self,
         client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
     ):
-        # Test without authentication
+        _, headers = make_org_user(role=None)
+        response = await client.get(
+            app.url_path_for("list_project_volunteers", project_id=default_project_id),
+            headers=headers,
+        )
+        assert response.status_code == 403
+
+
+class TestCreateVolunteer:
+    async def test_requires_auth(self, client: AsyncClient):
         response = await client.post(
             app.url_path_for("create_volunteer", project_id=default_project_id)
         )
         assert response.status_code == 401
 
-    async def test_create_volunteer_invalid_project_id(
+    async def test_invalid_project_id(
         self,
         client: AsyncClient,
-        default_user_headers: dict,
+        make_org_user,
     ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
         payload = {
             "name": "Lancelot",
             "email": "lancelot@dulac.fr",
             "phone_number": "03 14 15 92 65",
         }
-        # test invalid project_id
         response = await client.post(
             app.url_path_for("create_volunteer", project_id=uuid.uuid4()),
             json=payload,
-            headers=default_user_headers,
+            headers=headers,
         )
         assert response.status_code == 404
 
-    async def test_create_volunteer_invalid_payload(
+    async def test_invalid_payload(
         self,
         client: AsyncClient,
-        default_user_headers: dict,
+        default_public_project: Project,
+        make_org_user,
     ):
-        payload = {
-            "email": "lancelot@dulac.fr",
-            "phone_number": "03 14 15 92 65",
-        }
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        payload = {"email": "lancelot@dulac.fr", "phone_number": "03 14 15 92 65"}
         response = await client.post(
             app.url_path_for("create_volunteer", project_id=default_project_id),
             json=payload,
-            headers=default_user_headers,
+            headers=headers,
         )
         assert response.status_code == 422
 
-    async def test_create_volunteer_valid_payload(
+    @pytest.mark.parametrize("role", [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE])
+    async def test_write_roles_create(
         self,
         client: AsyncClient,
         default_public_project: Project,
-        default_user_headers: dict,
+        make_org_user,
         session: Session,
+        role,
     ):
+        _, headers = make_org_user(role=role)
         payload = {
             "name": "Lancelot",
             "email": "lancelot@dulac.fr",
@@ -122,25 +136,49 @@ class TestCreateVolunteer:
         response = await client.post(
             app.url_path_for("create_volunteer", project_id=default_project_id),
             json=payload,
-            headers=default_user_headers,
+            headers=headers,
         )
         assert response.status_code == 200
         assert response.json()["id"] != default_project_id
         assert response.json()["name"] == "Lancelot"
         assert response.json()["comment"] == ""
-        result = session.execute(
-            select(Volunteer).where(Volunteer.project_id == default_project_id)
+
+        volunteers = (
+            session.execute(select(Volunteer).where(Volunteer.project_id == default_project_id))
+            .scalars()
+            .all()
         )
-        volunteers = result.scalars().all()
         assert len(volunteers) > 1
 
-    async def test_create_volunteer_with_slots(
+    @pytest.mark.parametrize("role", WRITE_FORBIDDEN_ROLES)
+    async def test_read_only_roles_forbidden(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        role,
+    ):
+        _, headers = make_org_user(role=role)
+        payload = {
+            "name": "Lancelot",
+            "email": "lancelot@dulac.fr",
+            "phone_number": "03 14 15 92 65",
+        }
+        response = await client.post(
+            app.url_path_for("create_volunteer", project_id=default_project_id),
+            json=payload,
+            headers=headers,
+        )
+        assert response.status_code == 403
+
+    async def test_create_with_slots(
         self,
         client: AsyncClient,
         default_public_project: Project,
-        default_user_headers: dict,
+        make_org_user,
         session: Session,
     ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
         payload = {
             "name": "Lancelot",
             "email": "lancelot@dulac.fr",
@@ -150,278 +188,284 @@ class TestCreateVolunteer:
         response = await client.post(
             app.url_path_for("create_volunteer", project_id=default_project_id),
             json=payload,
-            headers=default_user_headers,
+            headers=headers,
         )
         assert response.status_code == 200
-        result = session.execute(
-            select(Volunteer).where(Volunteer.project_id == default_project_id)
+        volunteers = (
+            session.execute(select(Volunteer).where(Volunteer.project_id == default_project_id))
+            .scalars()
+            .all()
         )
-        volunteers = result.scalars().all()
-        assert len(volunteers) > 1
-        assert len(volunteers[0].slots) == 1
-
-
-async def test_create_volunteer_comment(
-    client: AsyncClient,
-    default_public_project: Project,
-    default_user_headers: dict,
-    session: Session,
-):
-    payload = {
-        "name": "Lancelot",
-        "email": "lancelot@dulac.fr",
-        "phone_number": "03 14 15 92 65",
-        "comment": "it's a knight",
-    }
-    # Test normal payload
-    response = await client.post(
-        app.url_path_for("create_volunteer", project_id=default_project_id),
-        json=payload,
-        headers=default_user_headers,
-    )
-    assert response.status_code == 200
-    assert response.json()["id"] != default_project_id
-    assert response.json()["name"] == "Lancelot"
-    assert response.json()["comment"] != ""
-    new_id = response.json()["id"]
-    result = session.execute(select(Volunteer).where(Volunteer.id == new_id))
-    volunteer = result.scalar_one_or_none()
-    assert volunteer is not None
-    assert volunteer.comment == "it's a knight"
-
-
-async def test_update_volunteer(
-    client: AsyncClient,
-    default_public_project: Project,
-    default_user_headers: dict,
-    session: Session,
-):
-    # Test without autentication
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
+        new_volunteer = [v for v in volunteers if v.id != default_volunteer_id][0]
+        assert len(new_volunteer.slots) == 1
+
+    async def test_create_with_comment(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        session: Session,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        payload = {
+            "name": "Lancelot",
+            "email": "lancelot@dulac.fr",
+            "phone_number": "03 14 15 92 65",
+            "comment": "it's a knight",
+        }
+        response = await client.post(
+            app.url_path_for("create_volunteer", project_id=default_project_id),
+            json=payload,
+            headers=headers,
         )
-    )
-    assert response.status_code == 401
-
-    payload = {
-        "name": "Lancelot",
-        "email": "lancelot@dulac.fr",
-        "phone_number": "03 14 15 92 65",
-        "automatic_sms": False,
-        "comment": "new comment",
-    }
-
-    # test invalid project_id
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=uuid.uuid4(),
-            volunteer_id=default_volunteer_id,
-        ),
-        json=payload,
-        headers=default_user_headers,
-    )
-    assert response.status_code == 404
+        assert response.status_code == 200
+        assert response.json()["comment"] != ""
+        volunteer = session.get(Volunteer, response.json()["id"])
+        assert volunteer.comment == "it's a knight"
 
-    # test invalid volunteer_id
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=default_project_id,
-            volunteer_id=uuid.uuid4(),
-        ),
-        json=payload,
-        headers=default_user_headers,
-    )
-    assert response.status_code == 404
 
-    # Test normal payload
+class TestUpdateVolunteer:
+    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
+        response = await client.post(
+            app.url_path_for(
+                "update_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+            )
+        )
+        assert response.status_code == 401
 
-    for k, v in payload.items():
+    async def test_invalid_project_id(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        payload = {"name": "Lancelot", "email": "l@dulac.fr", "phone_number": "0314159265"}
+        response = await client.post(
+            app.url_path_for(
+                "update_volunteer", project_id=uuid.uuid4(), volunteer_id=default_volunteer_id
+            ),
+            json=payload,
+            headers=headers,
+        )
+        assert response.status_code == 404
+
+    async def test_invalid_volunteer_id(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        payload = {"name": "Lancelot", "email": "l@dulac.fr", "phone_number": "0314159265"}
         response = await client.post(
             app.url_path_for(
-                "update_volunteer",
-                project_id=default_project_id,
-                volunteer_id=default_volunteer_id,
+                "update_volunteer", project_id=default_project_id, volunteer_id=uuid.uuid4()
             ),
-            json={k: v},
-            headers=default_user_headers,
+            json=payload,
+            headers=headers,
         )
+        assert response.status_code == 404
+
+    @pytest.mark.parametrize("role", [OrgRole.ORG_ADMIN, OrgRole.RESPO_BENEVOLE])
+    async def test_write_roles_update_each_field(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        role,
+    ):
+        _, headers = make_org_user(role=role)
+        payload = {
+            "name": "Lancelot",
+            "email": "lancelot@dulac.fr",
+            "phone_number": "03 14 15 92 65",
+            "automatic_sms": False,
+            "comment": "new comment",
+        }
+        for k, v in payload.items():
+            response = await client.post(
+                app.url_path_for(
+                    "update_volunteer",
+                    project_id=default_project_id,
+                    volunteer_id=default_volunteer_id,
+                ),
+                json={k: v},
+                headers=headers,
+            )
+            assert response.status_code == 200
+            assert response.json()["id"] == default_volunteer_id
+            assert response.json()[k] == v
+
+    @pytest.mark.parametrize("role", WRITE_FORBIDDEN_ROLES)
+    async def test_read_only_roles_forbidden(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        role,
+    ):
+        _, headers = make_org_user(role=role)
+        response = await client.post(
+            app.url_path_for(
+                "update_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+            ),
+            json={"comment": "hijacked"},
+            headers=headers,
+        )
+        assert response.status_code == 403
+
+    async def test_update_slots(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        session: Session,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        url = app.url_path_for(
+            "update_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+        )
+
+        response = await client.post(url, json={"slots": []}, headers=headers)
         assert response.status_code == 200
-        assert response.json()["id"] == default_volunteer_id
-        assert response.json()[k] == v
-
-
-async def test_update_volunteer_slots(
-    client: AsyncClient,
-    default_public_project: Project,
-    default_user_headers: dict,
-    session: Session,
-):
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
-        ),
-        json={"slots": []},
-        headers=default_user_headers,
-    )
+        slot = session.execute(select(Slot).where(Slot.id == default_slot_id)).scalars().first()
+        assert slot.volunteers_id == []
 
-    assert response.status_code == 200
-    result = session.execute(select(Slot).where(Slot.id == default_slot_id))
-    slot = result.scalars().first()
-    assert slot is not None
-    assert slot.volunteers_id == []
+        response = await client.post(url, json={"slots": [default_slot_id]}, headers=headers)
+        assert response.status_code == 200
+        session.refresh(slot)
+        assert slot.volunteers_id == [default_volunteer_id]
 
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
-        ),
-        json={"slots": [default_slot_id]},
-        headers=default_user_headers,
-    )
+        response = await client.post(url, json={"slots": [str(uuid.uuid4())]}, headers=headers)
+        assert response.status_code == 400
 
-    assert response.status_code == 200
-    session.refresh(slot)
-    assert slot is not None
-    assert slot.volunteers_id == [default_volunteer_id]
+        response = await client.post(url, json={"slots": ["not uuid str"]}, headers=headers)
+        assert response.status_code == 422
 
-    # An invalid slot list
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
-        ),
-        json={"slots": [str(uuid.uuid4())]},
-        headers=default_user_headers,
-    )
 
-    assert response.status_code == 400
+class TestDeleteVolunteer:
+    async def test_requires_auth(self, client: AsyncClient, default_public_project: Project):
+        response = await client.delete(
+            app.url_path_for(
+                "delete_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+            )
+        )
+        assert response.status_code == 401
 
-    # An invalid slot list
-    response = await client.post(
-        app.url_path_for(
-            "update_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
-        ),
-        json={"slots": ["not uuid str"]},
-        headers=default_user_headers,
-    )
+    @pytest.mark.parametrize("role", WRITE_FORBIDDEN_ROLES)
+    async def test_read_only_roles_forbidden(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        session: Session,
+        role,
+    ):
+        _, headers = make_org_user(role=role)
+        response = await client.delete(
+            app.url_path_for(
+                "delete_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+            ),
+            headers=headers,
+        )
+        assert response.status_code == 403
+        assert session.get(Volunteer, default_volunteer_id) is not None
 
-    assert response.status_code == 422
+    async def test_org_admin_deletes(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        session: Session,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        response = await client.delete(
+            app.url_path_for(
+                "delete_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+            ),
+            headers=headers,
+        )
+        assert response.status_code == 200
+        assert session.get(Volunteer, default_volunteer_id) is None
 
+        slot = session.execute(select(Slot).where(Slot.id == default_slot_id)).scalars().first()
+        assert default_volunteer_id not in slot.volunteers_id
 
-async def test_delete_volunteer(
-    client: AsyncClient,
-    default_user_headers: dict,
-    session: Session,
-    default_public_project: Project,
-):
-    # Fail deleting the project due to not logged in
-    response = await client.delete(
-        app.url_path_for(
-            "delete_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
+    async def test_idempotent_delete(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        url = app.url_path_for(
+            "delete_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
         )
-    )
-    assert response.status_code == 401
-    result = session.execute(select(Volunteer).where(Volunteer.id == default_volunteer_id))
-    volunteer = result.scalars().first()
-    assert volunteer is not None
-
-    # Proper deletion
-    response = await client.delete(
-        app.url_path_for(
-            "delete_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
-        ),
-        headers=default_user_headers,
-    )
-    assert response.status_code == 200
-    result = session.execute(select(Volunteer).where(Volunteer.id == default_volunteer_id))
-    volunteer = result.scalars().first()
-    assert volunteer is None
-
-    # check deletion is cascaded to slots
-    result = session.execute(select(Slot).where(Slot.id == default_slot_id))
-    slot: Slot | None = result.scalar_one_or_none()
-    assert slot is not None
-    assert default_volunteer_id not in slot.volunteers_id
-
-    # Idempotence test
-    response = await client.delete(
-        app.url_path_for(
-            "delete_volunteer",
-            project_id=default_project_id,
-            volunteer_id=default_volunteer_id,
-        ),
-        headers=default_user_headers,
-    )
-    assert response.status_code == 200
-
-
-async def test_delete_volunteer_any_uuid(
-    client: AsyncClient,
-    default_user_headers: dict,
-    session: Session,
-    default_public_project: Project,
-):
-    # can delete random uuid
-    response = await client.delete(
-        app.url_path_for(
-            "delete_volunteer", project_id=default_project_id, volunteer_id=uuid.uuid4()
-        ),
-        headers=default_user_headers,
-    )
-    assert response.status_code == 200
-
-    # Cannot delete non uuid string
-    response = await client.delete(
-        app.url_path_for(
-            "delete_volunteer", project_id=default_project_id, volunteer_id="not uidstr"
-        ),
-        headers=default_user_headers,
-    )
-    assert response.status_code == 422
-
-
-async def test_delete_volunteer_with_sms(
-    client: AsyncClient,
-    default_user_headers: dict,
-    session: Session,
-    default_public_project: Project,
-):
-    sms = Sms()
-    sms.project_id = default_project_id
-    sms.content = "coucou"
-    sms.phone_number = "02 66 66 66 66 66"
-    sms.volunteer_id = default_volunteer_id
-    session.add(sms)
-    session.commit()
-    session.refresh(sms)
-
-    response = await client.delete(
-        app.url_path_for(
-            "delete_volunteer",
+        response = await client.delete(url, headers=headers)
+        assert response.status_code == 200
+        response = await client.delete(url, headers=headers)
+        assert response.status_code == 200
+
+    async def test_delete_random_uuid(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+    ):
+        """Unlike delete_slot/delete_template, this still bulk-DELETEs
+        without an existence check via a Depends() resolver -- so a
+        nonexistent volunteer_id stays a silent 200, matching the original
+        behavior. If you want 404-on-missing consistency across all three
+        routers, this needs the same get()-then-check pattern slots.py uses."""
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        response = await client.delete(
+            app.url_path_for(
+                "delete_volunteer", project_id=default_project_id, volunteer_id=uuid.uuid4()
+            ),
+            headers=headers,
+        )
+        assert response.status_code == 200
+
+    async def test_invalid_uuid_format(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        response = await client.delete(
+            app.url_path_for(
+                "delete_volunteer", project_id=default_project_id, volunteer_id="not uidstr"
+            ),
+            headers=headers,
+        )
+        assert response.status_code == 422
+
+    async def test_delete_cascades_sms(
+        self,
+        client: AsyncClient,
+        default_public_project: Project,
+        make_org_user,
+        session: Session,
+    ):
+        _, headers = make_org_user(role=OrgRole.ORG_ADMIN)
+        sms = Sms(
             project_id=default_project_id,
+            content="coucou",
+            phone_number="02 66 66 66 66 66",
             volunteer_id=default_volunteer_id,
-        ),
-        headers=default_user_headers,
-    )
-    assert response.status_code == 200
-    # Volunteer must be deleted
-    result = session.execute(select(Volunteer).where(Volunteer.id == default_volunteer_id))
-    assert result.scalar_one_or_none() is None
+        )
+        session.add(sms)
+        session.commit()
+        sms_id = sms.id
 
-    # Sms must be deleted
-    assert session.execute(select(Sms).where(Sms.id == sms.id)).scalar_one_or_none() is None
+        response = await client.delete(
+            app.url_path_for(
+                "delete_volunteer", project_id=default_project_id, volunteer_id=default_volunteer_id
+            ),
+            headers=headers,
+        )
+        assert response.status_code == 200
+        assert session.get(Volunteer, default_volunteer_id) is None
+        assert session.execute(select(Sms).where(Sms.id == sms_id)).scalar_one_or_none() is None