From d87b485d4224f68b30643797d345d7350c0b9453 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 21 Aug 2026 15:31:13 +0200 Subject: [PATCH 1/2] fix: permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions (#269) --- .../permissions/contracts/schemas.py | 2 +- .../tests/test_permissions_module.py | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/modules/permissions/permissions/contracts/schemas.py b/modules/permissions/permissions/contracts/schemas.py index d3e0a314..fb390b1a 100644 --- a/modules/permissions/permissions/contracts/schemas.py +++ b/modules/permissions/permissions/contracts/schemas.py @@ -35,7 +35,7 @@ class RolePermissionsOut(SQLModel): class RolePermissionsUpdate(SQLModel): """Replace the full set of permission keys assigned to a role.""" - permissions: list[str] = Field(default_factory=list) + permissions: list[str] class UserOut(SQLModel): diff --git a/modules/permissions/tests/test_permissions_module.py b/modules/permissions/tests/test_permissions_module.py index de238616..38029c30 100644 --- a/modules/permissions/tests/test_permissions_module.py +++ b/modules/permissions/tests/test_permissions_module.py @@ -245,6 +245,37 @@ async def test_put_and_get_role_permissions( assert fetch.status_code == 200 assert fetch.json()["permissions"] == [PERM_VIEW] + async def test_put_requires_permissions_field( + self, authenticated_client: httpx.AsyncClient, app: FastAPI + ): + from users.constants import USER_ROLE_ID, USER_ROLE_NAME + from users.models import Role + + async with app.state.sm.db.session_factory() as db: + if await db.get(Role, USER_ROLE_ID) is None: + db.add(Role(id=USER_ROLE_ID, name=USER_ROLE_NAME, description="Standard user")) + await db.commit() + + seeded = await authenticated_client.put( + f"/api/permissions/roles/{USER_ROLE_ID}", + json={"permissions": [PERM_VIEW]}, + ) + assert seeded.status_code == 200 + + missing = await authenticated_client.put(f"/api/permissions/roles/{USER_ROLE_ID}", json={}) + assert missing.status_code == 422 + + unchanged = await authenticated_client.get(f"/api/permissions/roles/{USER_ROLE_ID}") + assert unchanged.status_code == 200 + assert unchanged.json()["permissions"] == [PERM_VIEW] + + cleared = await authenticated_client.put( + f"/api/permissions/roles/{USER_ROLE_ID}", + json={"permissions": []}, + ) + assert cleared.status_code == 200 + assert cleared.json()["permissions"] == [] + async def test_put_missing_role_returns_404(self, authenticated_client: httpx.AsyncClient): resp = await authenticated_client.put( f"/api/permissions/roles/{uuid.uuid4()}", From 02a7497c13a8896ae4f9b8f2e03b44424ee34afa Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sat, 22 Aug 2026 17:23:19 +0200 Subject: [PATCH 2/2] fix: address review feedback for #269 --- .../tests/test_permissions_module.py | 31 -------------- .../tests/test_permissions_role_update.py | 40 +++++++++++++++++++ 2 files changed, 40 insertions(+), 31 deletions(-) create mode 100644 modules/permissions/tests/test_permissions_role_update.py diff --git a/modules/permissions/tests/test_permissions_module.py b/modules/permissions/tests/test_permissions_module.py index 38029c30..de238616 100644 --- a/modules/permissions/tests/test_permissions_module.py +++ b/modules/permissions/tests/test_permissions_module.py @@ -245,37 +245,6 @@ async def test_put_and_get_role_permissions( assert fetch.status_code == 200 assert fetch.json()["permissions"] == [PERM_VIEW] - async def test_put_requires_permissions_field( - self, authenticated_client: httpx.AsyncClient, app: FastAPI - ): - from users.constants import USER_ROLE_ID, USER_ROLE_NAME - from users.models import Role - - async with app.state.sm.db.session_factory() as db: - if await db.get(Role, USER_ROLE_ID) is None: - db.add(Role(id=USER_ROLE_ID, name=USER_ROLE_NAME, description="Standard user")) - await db.commit() - - seeded = await authenticated_client.put( - f"/api/permissions/roles/{USER_ROLE_ID}", - json={"permissions": [PERM_VIEW]}, - ) - assert seeded.status_code == 200 - - missing = await authenticated_client.put(f"/api/permissions/roles/{USER_ROLE_ID}", json={}) - assert missing.status_code == 422 - - unchanged = await authenticated_client.get(f"/api/permissions/roles/{USER_ROLE_ID}") - assert unchanged.status_code == 200 - assert unchanged.json()["permissions"] == [PERM_VIEW] - - cleared = await authenticated_client.put( - f"/api/permissions/roles/{USER_ROLE_ID}", - json={"permissions": []}, - ) - assert cleared.status_code == 200 - assert cleared.json()["permissions"] == [] - async def test_put_missing_role_returns_404(self, authenticated_client: httpx.AsyncClient): resp = await authenticated_client.put( f"/api/permissions/roles/{uuid.uuid4()}", diff --git a/modules/permissions/tests/test_permissions_role_update.py b/modules/permissions/tests/test_permissions_role_update.py new file mode 100644 index 00000000..e536c6ee --- /dev/null +++ b/modules/permissions/tests/test_permissions_role_update.py @@ -0,0 +1,40 @@ +"""Regression tests for role permission update payloads.""" + +from __future__ import annotations + +import httpx +from fastapi import FastAPI +from permissions.constants import PERM_VIEW + + +class TestRolePermissionsAPI: + async def test_put_requires_permissions_field( + self, authenticated_client: httpx.AsyncClient, app: FastAPI + ): + from users.constants import USER_ROLE_ID, USER_ROLE_NAME + from users.models import Role + + async with app.state.sm.db.session_factory() as db: + if await db.get(Role, USER_ROLE_ID) is None: + db.add(Role(id=USER_ROLE_ID, name=USER_ROLE_NAME, description="Standard user")) + await db.commit() + + seeded = await authenticated_client.put( + f"/api/permissions/roles/{USER_ROLE_ID}", + json={"permissions": [PERM_VIEW]}, + ) + assert seeded.status_code == 200 + + missing = await authenticated_client.put(f"/api/permissions/roles/{USER_ROLE_ID}", json={}) + assert missing.status_code == 422 + + unchanged = await authenticated_client.get(f"/api/permissions/roles/{USER_ROLE_ID}") + assert unchanged.status_code == 200 + assert unchanged.json()["permissions"] == [PERM_VIEW] + + cleared = await authenticated_client.put( + f"/api/permissions/roles/{USER_ROLE_ID}", + json={"permissions": []}, + ) + assert cleared.status_code == 200 + assert cleared.json()["permissions"] == []