fix(security): gate every ServiceCatalogController route on the services permission

The controller carried only IS_AUTHENTICATED_FULLY on the class and none of its
15 routes checked a permission. A secretary whose owner had turned `services`
fully off could still create, rename and delete service categories, build item
groups, replace group members, and rewrite service relations and per-branch
overrides.

Scope is intra-tenant privilege escalation, not IDOR: owned() and requireItem()
already resolve every uuid against the caller's active environment, so no data
crossed tenants.

Gating is per-action (view/create/update/delete) and reuses denyServices() from
ClinicServiceController in the same domain, so a secretary with `update` cannot
create or delete. The call is the first statement in every action, before
requireCategory/requireItem — placed after, an unknown uuid would answer 404 and
leak whether the record exists.

An earlier note claimed these endpoints were consumed by the booking flow and so
could not be closed. That was wrong. service-selection/validate, the group routes
and the relation routes have no consumer in any of the three API clients, and the
sibling controller already puts every service read behind services.view — the
booking modal reads service-items through it — so any flow needing services
already needed the permission.

The docs claimed appointment_settings.* for the includes routes, which was never
enforced either; corrected to services.*.

The test loops the whole route list rather than sampling, and a guard asserts the
count of #[Route( equals the count of denyServices( so a future ungated route
fails here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
hamed
2026-08-07 18:46:16 +03:30
co-authored by Claude Opus 5
parent be93e15bad
commit 52c45443c5
4 changed files with 258 additions and 4 deletions
@@ -3,6 +3,8 @@
namespace App\ClinicService\Controller;
use App\Auth\Entity\User;
use App\Clinic\Security\ClinicDoctorAccessChecker;
use App\Secretary\Security\SecretaryAccessChecker;
use App\Doctor\Service\AddressResolver;
use App\ClinicService\Entity\CatalogCategory;
use App\ClinicService\Entity\CatalogCategoryInclude;
@@ -55,13 +57,30 @@ class ServiceCatalogController extends BaseController
private readonly CatalogCategoryIncludeRepository $includes,
private readonly CategoryClosureResolver $closure,
private readonly EntityManagerInterface $em,
private readonly SecretaryAccessChecker $secretaryAccess,
private readonly ClinicDoctorAccessChecker $clinicDoctorAccess,
) {}
/**
* گِیتِ ترکیبی: منشی + پزشکِ عضوِ کلینیک (هرکدام فقط نقشِ خودش را محدود می‌کند).
* قرینهٔ ClinicServiceController::denyServices — همان دامنه، همان معنا.
*
* پیش از هر کار دیگری صدا زده می‌شود؛ اگر بعد از requireCategory بیاید،
* ۴۰۴/۴۲۲ جای ۴۰۳ برمی‌گردد و وجود/نبودِ رکورد لو می‌رود.
*/
private function denyServices(User $user, string $action): void
{
$this->secretaryAccess->denyUnlessGranted($user, 'services', $action);
$this->clinicDoctorAccess->denyUnlessGranted($user, 'services', $action);
}
// ── دستهٔ درختی ─────────────────────────────────────────────────────────
#[Route('/api/v1/service-categories/tree', name: 'service_category_tree', methods: ['GET'])]
public function tree(#[CurrentUser] User $user): JsonResponse
{
$this->denyServices($user, 'view');
[$entityType, $entityId] = $this->branches->pair($user);
// کل درخت با یک کوئری خوانده و در PHP بسته می‌شود.
@@ -89,6 +108,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-category', name: 'service_category_create', methods: ['POST'])]
public function createCategory(#[CurrentUser] User $user, Request $request): JsonResponse
{
$this->denyServices($user, 'create');
$data = json_decode($request->getContent(), true);
$name = is_array($data) && is_string($data['name'] ?? null) ? trim($data['name']) : '';
@@ -126,6 +147,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-category/{uuid}', name: 'service_category_update', methods: ['PATCH'])]
public function updateCategory(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'update');
$data = json_decode($request->getContent(), true);
$category = $this->requireCategory($user, $uuid);
@@ -153,6 +176,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-category/{uuid}', name: 'service_category_delete', methods: ['DELETE'])]
public function deleteCategory(#[CurrentUser] User $user, string $uuid): JsonResponse
{
$this->denyServices($user, 'delete');
$category = $this->requireCategory($user, $uuid);
if ($this->categories->countChildren($category) > 0) {
@@ -176,6 +201,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-category/{uuid}/includes', name: 'service_category_includes', methods: ['GET'])]
public function listIncludes(#[CurrentUser] User $user, string $uuid): JsonResponse
{
$this->denyServices($user, 'view');
$category = $this->requireCategory($user, $uuid);
return $this->success(array_map(
@@ -187,6 +214,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-category/{uuid}/includes', name: 'service_category_include_add', methods: ['POST'])]
public function addInclude(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'create');
$data = json_decode($request->getContent(), true);
$childUuid = is_array($data) && is_string($data['child_category_uuid'] ?? null) ? trim($data['child_category_uuid']) : '';
@@ -213,6 +242,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-category/{uuid}/includes/{childUuid}', name: 'service_category_include_remove', methods: ['DELETE'])]
public function removeInclude(#[CurrentUser] User $user, string $uuid, string $childUuid): JsonResponse
{
$this->denyServices($user, 'delete');
$edge = $this->includes->findEdge(
$this->requireCategory($user, $uuid),
$this->requireCategory($user, $childUuid),
@@ -231,6 +262,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-item/{uuid}/groups', name: 'service_item_groups', methods: ['GET'])]
public function listGroups(#[CurrentUser] User $user, string $uuid): JsonResponse
{
$this->denyServices($user, 'view');
$service = $this->requireItem($user, $uuid);
return $this->success(array_map(
@@ -242,6 +275,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-item/{uuid}/groups', name: 'service_item_group_create', methods: ['POST'])]
public function createGroup(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'create');
$data = json_decode($request->getContent(), true);
$service = $this->requireItem($user, $uuid);
$name = is_array($data) && is_string($data['name'] ?? null) ? trim($data['name']) : '';
@@ -266,6 +301,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/item-group/{uuid}', name: 'item_group_update', methods: ['PATCH'])]
public function updateGroup(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'update');
$data = json_decode($request->getContent(), true);
$group = $this->requireGroup($user, $uuid);
@@ -291,6 +328,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/item-group/{uuid}', name: 'item_group_delete', methods: ['DELETE'])]
public function deleteGroup(#[CurrentUser] User $user, string $uuid): JsonResponse
{
$this->denyServices($user, 'delete');
$this->em->remove($this->requireGroup($user, $uuid));
$this->em->flush();
@@ -301,6 +340,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/item-group/{uuid}/items', name: 'item_group_items_replace', methods: ['PUT'])]
public function replaceGroupItems(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'update');
$data = json_decode($request->getContent(), true);
if (!is_array($data) || !is_array($data['items'] ?? null)) {
@@ -343,6 +384,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-item/{uuid}/relations', name: 'service_item_relations_replace', methods: ['PUT'])]
public function replaceRelations(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'update');
$data = json_decode($request->getContent(), true);
if (!is_array($data) || !is_array($data['relations'] ?? null)) {
@@ -431,6 +474,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-item/{uuid}/branch-overrides', name: 'service_item_overrides_replace', methods: ['PUT'])]
public function replaceBranchOverrides(#[CurrentUser] User $user, string $uuid, Request $request): JsonResponse
{
$this->denyServices($user, 'update');
$data = json_decode($request->getContent(), true);
if (!is_array($data) || !is_array($data['overrides'] ?? null)) {
@@ -487,6 +532,8 @@ class ServiceCatalogController extends BaseController
#[Route('/api/v1/service-selection/validate', name: 'service_selection_validate', methods: ['POST'])]
public function validateSelection(#[CurrentUser] User $user, Request $request): JsonResponse
{
$this->denyServices($user, 'view');
$data = json_decode($request->getContent(), true);
if (!is_array($data) || !is_array($data['item_uuids'] ?? null)) {