fix(web-api): CORS preflight OPTIONS до CorsMiddleware

[Ibochkarev]

Описание

Браузерный CORS preflight (OPTIONS) к Web API получал 405 Method not allowed: FastRoute отдавал METHOD_NOT_ALLOWED до запуска group middleware, и CorsMiddleware не успевал ответить.

Теперь Router::dispatch() на OPTIONS для известных storefront-путей (/api/v1/*) прогоняет только middleware найденного маршрута. Handler не вызывается — preflight не триггерит POST/GET side effects. CorsMiddleware возвращает Response 200 вместо exit, чтобы Router и PHPUnit могли обработать ответ.

Closes #634

Тип изменений

• Исправление бага (non-breaking change)

Связанные Issues

Closes #634

Как это было протестировано?

cd core/components/minishop3
composer ci:php   # exit 0 — php -l, smoke (89), PHPUnit 260 tests

Добавлены HeadlessStorefrontCorsRouterTest: OPTIONS на /api/v1/health, POST-only /api/v1/cart/add, неизвестный path → 404, handler не вызывается без CorsMiddleware.

• Автоматические тесты (composer ci:php)

Конфигурация тестирования:

• MiniShop3: beta
• PHP: 8.4.17 (локально)

Чеклист

• Код соответствует стилю проекта
• Изменения не ломают существующую функциональность
• Лексиконы (не требуются)
• CHANGELOG (релиз maintainer)

Дополнительные заметки

Кастомные роуты в ms3.routes.d/web без CorsMiddleware по-прежнему получат 405 на OPTIONS (handler не выполняется). Для headless-витрины аддон должен вешать тот же CORS stack, что и web.php.

[AgelxNash]

Этот PR включён в тестовую интеграционную сборку всех открытых PR MiniShop3: AgelxNash/MiniShop3, ветка integration/open-prs-20260906 (28/28 открытых).

Сборка нужна, чтобы проверить совместимость взаимозависимых серий PR до их мержа — при последовательном слиянии они конфликтуют друг с другом. Это не ревью и не конкурирующий PR: авторство сохранено (1 PR = 1 коммит с исходным автором), ветка пересобирается по мере обновления PR.

Как вошёл в сборку: Слился чисто.

[AgelxNash]

Отличная работа! Желаю этому PR быстрого мержа и ни одного конфликта 🙌

[biz87]

Диагноз верный, проверил по коду: до правки ни один роут в config/routes/web.php не регистрирует OPTIONS, поэтому Dispatcher::dispatch('OPTIONS', ...) всегда возвращал METHOD_NOT_ALLOWED, Router::dispatch() отдавал 405 без единого вызова middleware, и CorsMiddleware с его exit() был для настоящего браузерного preflight мёртвым кодом. Замена exit на Response тоже корректна — сигнатура Response::success() реальная.

Локальный прогон на ветке, смерженной с актуальной beta: php -l чисто по 628 файлам, smoke 89/89, PHPUnit 260 (9 skipped — mysql-группа без DSN), --testsuite WebApi 21/21, PHPStan по CorsMiddleware.php и Router.php — 0 ошибок. Всё сходится с описанием PR.

Хардненинг 1.13.0 не откачен — src/Utils/CorsConfig.php в диффе отсутствует, принудительный сброс allow_credentials при wildcard-origin на месте. Эхо Origin по-прежнему только внутри проверки по списку разрешённых.

Но просьба доработать одну вещь, прежде чем вливать.

dispatchOptionsPreflight() открывает побочные эффекты на неаутентифицированном OPTIONS

src/Router/Router.php:408-434 прогоняет весь middleware-стек найденного роута в поисках того, кто ответит на preflight. Для наших собственных роутов это безопасно: в config/routes/web.php:340 группа /api/v1 объявлена как [$corsMiddleware, $rateLimitMiddleware, $serviceCheckMiddleware], CORS первый и замыкает цепочку раньше остальных.

Проблема — в роутах, собранных по нашему же документированному шаблону. config/ms3.routes.d/web/example-addon.php.dist:47 вешает на группу [new TokenMiddleware($modx)], без CORS. В такой конфигурации preflight доходит до TokenMiddleware, а тот на non-public роуте при отсутствии валидного токена делает безусловный минт (src/Middleware/TokenMiddleware.php, ветка if (!$isPublic) { $result = $tokenService->generateCustomerToken(); ... }): создаётся анонимный токен, стартует сессия, идёт запись в БД через TokenService::persistApiToken.

То есть OPTIONS без токена и куки, с произвольным Origin, никак не связанным со списком разрешённых, создаёт состояние. Итоговый статус всё равно 405, но эффект уже произошёл. Воспроизводится тестом, повторяющим паттерн из .dist: Origin: https://evil.example → в $_REQUEST['ms3_token'] появляется свежий токен.

Ключевое: это не унаследованное поведение, а новый вектор. До PR такой запрос отбивался на 405 до всякого middleware. В шаблоне аддона RateLimitMiddleware тоже не подключён, так что дешёвый флуд анонимными токенами и сессиями ничем не ограничен.

Варианты на выбор:

1. В dispatchOptionsPreflight(): если после прохода по цепочке ни один middleware не вернул Response, не считать это молчаливым успехом, а требовать наличия CorsMiddleware в стеке — иначе отдавать 405 до вызова middleware, как было раньше.
2. Либо явно задокументировать в example-addon.php.dist, что CorsMiddleware обязателен в любой группе с TokenMiddleware/RateLimitMiddleware — и поправить сам шаблон, раз он служит образцом.

Первый вариант надёжнее: он не полагается на то, что автор аддона прочитает документацию.

Ещё просьба — тест на этот случай. Нынешний testOptionsPreflightDoesNotInvokeHandlerWithoutCorsMiddleware (tests/Integration/WebApi/HeadlessStorefrontCorsRouterTest.php:74-95) берёт роут вовсе без middleware, то есть проверяет более слабое условие и эту дыру не ловит. Нужен именно «TokenMiddleware без CorsMiddleware + OPTIONS».

Мелочи

• Комментарий в dispatchOptionsPreflight() — «Never invokes the route handler: preflight must not trigger POST/GET side effects» — обещает больше, чем даёт код. Для handler'а верно, для middleware нет. Стоит переформулировать, иначе следующий читатель будет опираться на несуществующую гарантию.
• Выбор цепочки по allowedMethods[0]: сегодня в web.php нет путей с разными middleware для разных методов, так что работает, но конструкция хрупкая на будущее.
• На неразрешённый Origin preflight отвечает 200 без Access-Control-Allow-Origin — браузер запрос всё равно заблокирует. Поведение унаследовано от прежнего безусловного exit, не регрессия, но раз ветку всё равно трогаем — можно отдавать 403.

Понимаю, что PR первый в очереди и держит браузерный headless целиком. Возвращаю не из-за качества самого фикса — он нужен и сделан правильно, — а потому что вместе с ним в релиз уедет путь, где неаутентифицированный запрос создаёт сессию. Правка небольшая, а цена ошибки высокая.

[Ibochkarev]

Адрес ревью:

1. dispatchOptionsPreflight: стек middleware запускается только если в нём есть CorsMiddleware; иначе сразу 405 без вызовов (как до PR). Side-effect TokenMiddleware на OPTIONS закрыт.
2. Тест testOptionsPreflightWithTokenMiddlewareButNoCorsDoesNotMintToken — паттерн из example-addon.php.dist + Origin: https://evil.example → 405, ms3_token не появляется.
3. Комментарий уточнён: handler не вызывается; middleware без CORS тоже не бегут.
4. Шаблон example-addon.php.dist — CORS первым в группе с TokenMiddleware.

Rebase на актуальную beta. composer ci:php зелёный (PHPUnit 276).

403 на disallowed Origin и выбор allowedMethods[0] не трогал — как в ревью, не регрессия / не блокер.

[biz87]

Первая половина фикса подтверждена независимо — собрал сценарий сам, не полагаясь на твой тест: паттерн из example-addon.php.dist (TokenMiddleware без CorsMiddleware), OPTIONS с Origin: https://evil.example, без токена и куки → 405, ms3_token не появляется. Стоковые роуты при этом не сломаны: разрешённый Origin получает 200 с корректными заголовками. CorsConfig::normalizeCorsConfig() вне диффа, сброс allow_credentials при wildcard на месте.

Числа после мержа с актуальной beta (за сутки влито 14 PR): smoke 94/94, PHPUnit 295 тестов / 731 assertion, --testsuite WebApi 29/29, PHPStan из vendor/bin — 0 ошибок.

Но защита оказалась неполной, и это следствие моей формулировки в прошлом возврате.

Гейт проверяет наличие CorsMiddleware, но не его позицию

Router::middlewareStackHasCors() перебирает весь массив и возвращает true, если CorsMiddleware найден в любой позиции. Реальная же защита держится не на этом, а на том, что CorsMiddleware::handle() успевает вернуть Response раньше, чем отработает TokenMiddleware.

Собрал тест с тем же паттерном, но с обратным порядком — [new TokenMiddleware($modx), new CorsMiddleware(['allowed_origins' => ['https://trusted.example']])], OPTIONS с Origin: https://evil.example, без токена и куки:

STATUS=200
ms3_token set=YES:anon-journey-1
handlerCalled=NO

Токен заминчен для неаутентифицированного запроса с недоверенного Origin. Гейт пройден — CorsMiddleware в массиве присутствует, — но TokenMiddleware стоит первым и успевает создать состояние. Это ровно тот сценарий из #634, который PR закрывает, просто триггер другой: не «CORS нет вовсе», а «CORS есть, но не первый».

Сейчас и config/routes/web.php, и example-addon.php.dist держат правильный порядок. Но это соглашение, а не инвариант: ничто не мешает автору стороннего аддона или будущему PR переставить их местами, и защита исчезнет молча — теста на такой случай нет.

Оговорюсь честно: в прошлом возврате я написал «требовать наличия CorsMiddleware в стеке». Ты реализовал ровно это. Неполнота — на мне, не на тебе.

Что предлагаю вместо

Проверять $middlewares[0] instanceof CorsMiddleware можно, но это лечит симптом: защита продолжает зависеть от порядка, просто теперь он валидируется. Мне кажется правильнее сделать порядок несущественным — короткое замыкание на OPTIONS внутри самих middleware, которые пишут состояние:

if (($_SERVER['REQUEST_METHOD'] ?? '') === 'OPTIONS') {
    return null; // preflight не создаёт состояния
}

Проверил: это касается не только TokenMiddleware (он вообще не смотрит на метод — ни одного упоминания REQUEST_METHOD в файле), но и RateLimitMiddleware, который на строке 59 делает $this->store->increment($key, $this->decaySeconds) тоже безусловно. То есть preflight сейчас расходует лимит запросов ещё до того, как кто-либо решит, отвечать ли на него.

Тогда инвариант формулируется не как «CORS должен быть первым», а как «на OPTIONS ничего не пишется» — и держится сам, без соглашений о порядке в каждом новом аддоне.

Заодно это приведёт код в соответствие с комментарием, который ты уже написал в Router.php: «TokenMiddleware / RateLimitMiddleware cannot mint tokens or open sessions on unauthenticated OPTIONS». Сейчас комментарий обещает больше, чем гарантирует код.

И тест на обратный порядок — чтобы регрессия ловилась в CI, а не при следующем ручном ревью.

Остальное вопросов не вызывает: шаблон аддона поправлен верно, middlewareStackHasCors() корректно работает через instanceof/is_a и переживёт подкласс CorsMiddleware из стороннего аддона, комментарий про preflight уточнён.

[Ibochkarev]

Спасибо — формулировка «наличие CorsMiddleware» действительно оставляла дыру при обратном порядке.

Сделано после rebase на актуальную beta (конфликтов не было):

1. TokenMiddleware / RateLimitMiddleware — в начале handle() на OPTIONS сразу return null (без mint токена / session и без increment квоты).
2. Тест testOptionsPreflightWithTokenBeforeCorsDoesNotMintToken — стек [Token, Cors], Origin: https://evil.example → 200, ms3_token нет.
3. Unit: testOptionsPreflightDoesNotConsumeQuota для rate limit.
4. Комментарий в Router::dispatchOptionsPreflight приведён к инварианту «на OPTIONS ничего не пишется», а не «CORS обязан быть первым».

Прогон: HeadlessStorefrontCorsRouterTest 8/8, RateLimitMiddlewareTest 4/4.

Готово к повторному ревью.