P1: Booker (любой JWT) может confirm чужую/свою заявку — нет ACL #53

Closed
opened 2026-07-22 10:34:04 +03:00 by cursor-ai · 3 comments
Owner

Проблема

PUT /v1/bookings/{id} с action=confirm не проверяет, что вызывающий — владелец календаря/события (или admin). В logic_booking:confirm_booking/2 аргумент пользователя помечен как _UserId и игнорируется: любой аутентифицированный пользователь может подтвердить любую pending-заявку.

Stage QA: booker (smoke-user) подтвердил свою заявку c6kW9Towqfe9y6hOcrVFHg на чужое событие ggek0yrBKg0YFwHUwvNCKg200 confirmed. Ожидалось 403.

Спека (EventHubBackSpec §2.3): «Владелец календаря может подтвердить или отклонить заявку»; участник может только отменить свою запись. Handler-комментарий тоже говорит «владельцем», но ACL в logic отсутствует.

Дополнительно: handler принимает action=decline, но в confirm_booking/3 есть только clause для confirmdecline может давать function_clause/500 вместо осмысленного ответа.

Влияние

Нарушение модели доверия для confirmation=manual: booker (и любой другой JWT) может сам подтвердить запись → обход ручного approve владельца, искажение capacity/статусов, ложный gate для отзывов (confirmed booking).

Ожидаемый результат

  • Confirm/decline: только владелец календаря события или admin → 200.
  • Booker / посторонний → 403.
  • Decline реализован явно (например, статус cancelled или отдельное правило по спеке) без function_clause.
  • Unit/API-тесты на ACL (booker denied, owner ok, admin ok).

Критерии приёмки

  • Booker PUT … action=confirm → 403
  • Owner PUT … action=confirm → 200, status=confirmed
  • Посторонний user → 403
  • Admin → 200 (если так принято в проекте)
  • action=decline работает по согласованному правилу + ACL как у confirm
  • Unit-тест: booker не может confirm (сейчас test_booking_event_full как раз вызывает confirm от participant — поправить)
  • Спека/swagger при необходимости уточнить endpoint PUT

Варианты решения

  1. Рекомендуется: в confirm_booking/2 (и decline) ACL как у list_event_bookings/2: event → calendar → owner_id =:= UserId orelse admin_utils:is_admin(UserId); иначе access_denied. Переиспользовать общий helper can_manage_event_bookings/2.
  2. Отдельная функция decide_booking(UserId, BookingId, Action) с явной матрицей ролей + тесты; handler только вызывает её.
  3. Проверка только «не booker» (запрет self-confirm) — слабо: чужой user всё ещё сможет confirm.

Файлы (подсказка)

  • src/logic/logic_booking.erl (confirm_booking/2, /3; сравнить с list_event_bookings/2)
  • src/handlers/handler_booking_by_id.erl (update_booking)
  • test/unit/logic_booking_tests.erl

Приоритет

P1 (по факту шире, чем «booker self-confirm»: любой auth user)

## Проблема `PUT /v1/bookings/{id}` с `action=confirm` **не проверяет**, что вызывающий — владелец календаря/события (или admin). В `logic_booking:confirm_booking/2` аргумент пользователя помечен как `_UserId` и **игнорируется**: любой аутентифицированный пользователь может подтвердить любую `pending`-заявку. Stage QA: booker (smoke-user) подтвердил свою заявку `c6kW9Towqfe9y6hOcrVFHg` на чужое событие `ggek0yrBKg0YFwHUwvNCKg` → **200 confirmed**. Ожидалось **403**. Спека (`EventHubBackSpec` §2.3): «Владелец календаря может подтвердить или отклонить заявку»; участник может только отменить свою запись. Handler-комментарий тоже говорит «владельцем», но ACL в logic отсутствует. Дополнительно: handler принимает `action=decline`, но в `confirm_booking/3` есть только clause для `confirm` — `decline` может давать `function_clause`/500 вместо осмысленного ответа. ## Влияние Нарушение модели доверия для `confirmation=manual`: booker (и любой другой JWT) может сам подтвердить запись → обход ручного approve владельца, искажение capacity/статусов, ложный gate для отзывов (confirmed booking). ## Ожидаемый результат - Confirm/decline: только владелец календаря события **или** admin → 200. - Booker / посторонний → **403**. - Decline реализован явно (например, статус `cancelled` или отдельное правило по спеке) без function_clause. - Unit/API-тесты на ACL (booker denied, owner ok, admin ok). ## Критерии приёмки - [ ] Booker `PUT … action=confirm` → 403 - [ ] Owner `PUT … action=confirm` → 200, status=confirmed - [ ] Посторонний user → 403 - [ ] Admin → 200 (если так принято в проекте) - [ ] `action=decline` работает по согласованному правилу + ACL как у confirm - [ ] Unit-тест: booker не может confirm (сейчас `test_booking_event_full` как раз вызывает confirm от participant — поправить) - [ ] Спека/swagger при необходимости уточнить endpoint PUT ## Варианты решения 1. **Рекомендуется:** в `confirm_booking/2` (и decline) ACL как у `list_event_bookings/2`: event → calendar → `owner_id =:= UserId orelse admin_utils:is_admin(UserId)`; иначе `access_denied`. Переиспользовать общий helper `can_manage_event_bookings/2`. 2. Отдельная функция `decide_booking(UserId, BookingId, Action)` с явной матрицей ролей + тесты; handler только вызывает её. 3. Проверка только «не booker» (запрет self-confirm) — **слабо**: чужой user всё ещё сможет confirm. ## Файлы (подсказка) - `src/logic/logic_booking.erl` (`confirm_booking/2`, `/3`; сравнить с `list_event_bookings/2`) - `src/handlers/handler_booking_by_id.erl` (`update_booking`) - `test/unit/logic_booking_tests.erl` ## Приоритет P1 (по факту шире, чем «booker self-confirm»: любой auth user)
cursor-ai added the Bug label 2026-07-22 10:34:04 +03:00
cursor-ai self-assigned this 2026-07-22 10:34:04 +03:00
Author
Owner

Реализуем вариант 1: ACL как у list_event_bookings/2 — confirm/decline только владелец календаря или admin; booker/посторонний → 403. Общий helper can_manage_event_bookings, правка decline при необходимости, обновление unit-тестов.

Реализуем **вариант 1**: ACL как у `list_event_bookings/2` — confirm/decline только владелец календаря или admin; booker/посторонний → 403. Общий helper `can_manage_event_bookings`, правка decline при необходимости, обновление unit-тестов.
Author
Owner

Готово.

Что сделано

  • ACL на confirm/decline как у list_event_bookings/2: только владелец календаря или admin.
  • Helper can_manage_event_bookings/2; action=declinecancelled.
  • Unit-тесты: owner OK, booker/stranger 403, admin OK, decline.

Тесты

  • logic_booking_tests: 18/0
  • booking_integration_tests: 4/0

Коммит

fdb08eb — pushed to master.

Спека уже описывала владельца календаря — правки EventHubSpec не нужны.

Готово. ## Что сделано - ACL на confirm/decline как у `list_event_bookings/2`: только владелец календаря или admin. - Helper `can_manage_event_bookings/2`; `action=decline` → `cancelled`. - Unit-тесты: owner OK, booker/stranger 403, admin OK, decline. ## Тесты - `logic_booking_tests`: 18/0 - `booking_integration_tests`: 4/0 ## Коммит `fdb08eb` — pushed to `master`. Спека уже описывала владельца календаря — правки EventHubSpec не нужны.
Author
Owner

Проверено на stage после деплоя fdb08eb (API build 153):

  • booker PUT /v1/bookings/:id action=confirm на свежую pending → 403 Access denied
  • задача остаётся закрытой
Проверено на stage после деплоя `fdb08eb` (API build 153): - booker `PUT /v1/bookings/:id` action=confirm на свежую pending → **403 Access denied** - задача остаётся закрытой
Sign in to join this conversation.