Skip to content

Commit

Permalink
feat: Include CSRF protected index.php routes
Browse files Browse the repository at this point in the history
Signed-off-by: provokateurin <[email protected]>
  • Loading branch information
provokateurin committed Nov 11, 2024
1 parent d7c135e commit e54e27d
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 13 deletions.
17 changes: 5 additions & 12 deletions generate-spec
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -558,7 +553,7 @@ foreach ($parsedRoutes as $key => $value) {
$classMethodInfo,
$isOCS,
$isCORS,
$isCSRFRequired,
$isNoCSRFRequired,
$isPublic,
);
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion src/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {
}
Expand Down
11 changes: 11 additions & 0 deletions tests/lib/Controller/RoutingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,15 @@ public function attributeOCS() {
public function attributeIndex() {
return DataResponse();
}

/**
* Index Route with CSRF required
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
*
* 200: Success
*/
#[FrontpageRoute(verb: 'PUT', url: '/attribute-index/{param}', requirements: ['param' => '[a-z]+'], defaults: ['param' => 'abc'], root: '/tests')]
public function csrfIndex() {
return DataResponse();
}
}
48 changes: 48 additions & 0 deletions tests/openapi-administration.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {}
}
}
}
}
}
}
},
Expand Down
48 changes: 48 additions & 0 deletions tests/openapi-full.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down

0 comments on commit e54e27d

Please sign in to comment.