Skip to content

fix(security): IDOR — PUT/DELETE /api/offices/:id теперь проверяет ownership#18

Merged
Br1Im merged 1 commit into
mainfrom
devin/1778714779-idor-offices
May 13, 2026
Merged

fix(security): IDOR — PUT/DELETE /api/offices/:id теперь проверяет ownership#18
Br1Im merged 1 commit into
mainfrom
devin/1778714779-idor-offices

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Закрывает уязвимость, упомянутую в Notes PR #15:

Было: PUT /api/offices/:officeId и DELETE /api/offices/:officeId принимали запрос от любого авторизованного пользователя — стоило только знать ID чужого офиса, и можно было его переименовать или удалить. Это IDOR (Insecure Direct Object Reference).

Стало: оба endpoint'а вызывают isOfficeOwner(req.user, officeId) — проверка offices.owner_id === req.user.id (или системная роль owner). Если не владелец → 403 Доступ запрещён.

Изменения

  • server/controllers/officeController.js:
    • Новый хелпер isOfficeOwner(user, officeId) — селектит owner_id из БД и сравнивает с req.user.id.
    • updateOffice — добавлен guard перед записью.
    • deleteOffice — добавлен guard перед удалением.

Тесты

server/__tests__/offices.test.js, +3 теста:

  1. PUT /api/offices/:id — чужак (другой директор) пытается обновить — 403, и реально в БД имя не поменялось.
  2. DELETE /api/offices/:id — владелец удаляет свой офис → 2xx, строка из offices исчезает.
  3. DELETE /api/offices/:id — чужак удаляет → 403, строка в БД на месте.

Полный backend Jest suite: 95/95 PASS.

Review & Testing Checklist for Human

  • Авторизоваться двумя директорами (каждый со своим офисом) и убедиться, что:
    • Свой офис можно переименовать/удалить.
    • При попытке PUT/DELETE на чужой officeId приходит 403.
  • Существующий UI-flow «настройки офиса» для владельца не сломан.

Notes

— Хелпер isOfficeOwner локальный для officeController.js. Если в будущем понадобится использовать его ещё где-то (например, для трансфера офиса, изменения плана и т.п.), можно вынести в utils/ensureOffice.js.
— Системная роль 'owner' (если она когда-то будет назначена админу всей системы) проходит проверку без БД-запроса.
— Этот PR не меняет права на GET (чтение) — там уже работает checkOfficeAccess.

Link to Devin session: https://app.devin.ai/sessions/514f368ad0194bf18d0327e62a88aeda
Requested by: @Br1Im

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@Br1Im Br1Im merged commit 004c211 into main May 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant