From e54e27d1662a9ab54d72d08332afb6d80d2ad4d7 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 11 Nov 2024 12:56:58 +0100 Subject: [PATCH] feat: Include CSRF protected index.php routes Signed-off-by: provokateurin --- generate-spec | 17 +++----- src/Route.php | 2 +- tests/lib/Controller/RoutingController.php | 11 +++++ tests/openapi-administration.json | 48 ++++++++++++++++++++++ tests/openapi-full.json | 48 ++++++++++++++++++++++ 5 files changed, 113 insertions(+), 13 deletions(-) diff --git a/generate-spec b/generate-spec index 5ebd5d1..d700c90 100755 --- a/generate-spec +++ b/generate-spec @@ -437,12 +437,7 @@ foreach ($parsedRoutes as $key => $value) { Logger::panic($routeName, 'Missing controller method'); } - $isCSRFRequired = !Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'NoCSRFRequired'); - if ($isCSRFRequired && !$isOCS) { - Logger::debug($routeName, 'Route ignored because of required CSRF in a non-OCS controller'); - continue; - } - + $isNoCSRFRequired = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'NoCSRFRequired'); $isCORS = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'CORS'); $isPublic = Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'PublicPage'); $isAdmin = !Helpers::classMethodHasAnnotationOrAttribute($methodFunction, 'NoAdminRequired') && !$isPublic; @@ -558,7 +553,7 @@ foreach ($parsedRoutes as $key => $value) { $classMethodInfo, $isOCS, $isCORS, - $isCSRFRequired, + $isNoCSRFRequired, $isPublic, ); } @@ -749,10 +744,8 @@ foreach ($routes as $scope => $scopeRoutes) { // Bearer auth is not allowed on CORS routes $security[] = ['bearer_auth' => []]; } - if (!$route->isCSRFRequired || $route->isOCS) { - // Add basic auth last, so it's only fallback if bearer is available - $security[] = ['basic_auth' => []]; - } + // Add basic auth last, so it's only fallback if bearer is available + $security[] = ['basic_auth' => []]; $operation = [ 'operationId' => $route->operationId, @@ -822,7 +815,7 @@ foreach ($routes as $scope => $scopeRoutes) { $parameters[] = $parameter; } - if ($route->isOCS) { + if ($route->isOCS || !$route->isNoCSRFRequired) { $parameters[] = [ 'name' => 'OCS-APIRequest', 'in' => 'header', diff --git a/src/Route.php b/src/Route.php index e886176..6e2a460 100644 --- a/src/Route.php +++ b/src/Route.php @@ -19,7 +19,7 @@ public function __construct( public ControllerMethod $controllerMethod, public bool $isOCS, public bool $isCORS, - public bool $isCSRFRequired, + public bool $isNoCSRFRequired, public bool $isPublic, ) { } diff --git a/tests/lib/Controller/RoutingController.php b/tests/lib/Controller/RoutingController.php index ca5c929..10c7777 100644 --- a/tests/lib/Controller/RoutingController.php +++ b/tests/lib/Controller/RoutingController.php @@ -39,4 +39,15 @@ public function attributeOCS() { public function attributeIndex() { return DataResponse(); } + + /** + * Index Route with CSRF required + * @return DataResponse, array{}> + * + * 200: Success + */ + #[FrontpageRoute(verb: 'PUT', url: '/attribute-index/{param}', requirements: ['param' => '[a-z]+'], defaults: ['param' => 'abc'], root: '/tests')] + public function csrfIndex() { + return DataResponse(); + } } diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index b7dd124..6e30b23 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -4367,6 +4367,54 @@ } } } + }, + "put": { + "operationId": "routing-csrf-index", + "summary": "Index Route with CSRF required", + "description": "This endpoint requires admin access", + "tags": [ + "routing" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "param", + "in": "path", + "required": true, + "schema": { + "type": "string", + "pattern": "^[a-z]+$", + "default": "abc" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Success", + "content": { + "application/json": { + "schema": {} + } + } + } + } } } }, diff --git a/tests/openapi-full.json b/tests/openapi-full.json index 6147403..4330e57 100644 --- a/tests/openapi-full.json +++ b/tests/openapi-full.json @@ -4517,6 +4517,54 @@ } } } + }, + "put": { + "operationId": "routing-csrf-index", + "summary": "Index Route with CSRF required", + "description": "This endpoint requires admin access", + "tags": [ + "routing" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "param", + "in": "path", + "required": true, + "schema": { + "type": "string", + "pattern": "^[a-z]+$", + "default": "abc" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Success", + "content": { + "application/json": { + "schema": {} + } + } + } + } } }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/default-admin-overwritten": {