diff --git a/lib/Controller/DsgvoController.php b/lib/Controller/DsgvoController.php index da343a0..40c2f83 100644 --- a/lib/Controller/DsgvoController.php +++ b/lib/Controller/DsgvoController.php @@ -11,7 +11,6 @@ use OCA\Mitgliederverwaltung\Service\ValidationException; use OCP\AppFramework\ApiController; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; -use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; @@ -58,7 +57,6 @@ class DsgvoController extends ApiController { * POST /api/v1/members/{id}/dsgvo-export * Body: { "password": "..." } */ - #[NoCSRFRequired] public function export(int $id): DataDownloadResponse|JSONResponse { try { $this->assertAdmin(); @@ -119,7 +117,6 @@ class DsgvoController extends ApiController { * DELETE /api/v1/members/{id}/dsgvo-delete * Body: { "confirm": true } */ - #[NoCSRFRequired] public function hardDelete(int $id): JSONResponse { try { $this->assertAdmin(); diff --git a/lib/Controller/ExportController.php b/lib/Controller/ExportController.php index 980bac3..7525ae4 100644 --- a/lib/Controller/ExportController.php +++ b/lib/Controller/ExportController.php @@ -155,7 +155,6 @@ class ExportController extends ApiController { * POST /api/v1/export/members/encrypted * Body: { "password": "...", "status": "aktiv" } */ - #[NoCSRFRequired] public function membersEncrypted(): DataDownloadResponse|JSONResponse { try { $password = $this->request->getParam('password'); @@ -204,7 +203,6 @@ class ExportController extends ApiController { * POST /api/v1/export/fees/encrypted * Body: { "password": "...", "year": 2026 } */ - #[NoCSRFRequired] public function feesEncrypted(): DataDownloadResponse|JSONResponse { try { $password = $this->request->getParam('password'); @@ -260,7 +258,6 @@ class ExportController extends ApiController { * POST /api/v1/export/birthdays/encrypted * Body: { "password": "...", "month": 4 } */ - #[NoCSRFRequired] public function birthdaysEncrypted(): DataDownloadResponse|JSONResponse { try { $password = $this->request->getParam('password'); @@ -354,7 +351,6 @@ class ExportController extends ApiController { * * Requires explicit confirmation because sensitive data is decrypted. */ - #[NoCSRFRequired] public function bundleSensitive(): DataDownloadResponse|JSONResponse { try { $confirm = $this->request->getParam('confirm'); @@ -448,7 +444,6 @@ class ExportController extends ApiController { * POST /api/v1/export/banking/encrypted * Body: { "password": "..." } */ - #[NoCSRFRequired] public function bankingEncrypted(): DataDownloadResponse|JSONResponse { try { // Permission check: banking visibility required diff --git a/lib/Controller/ReportController.php b/lib/Controller/ReportController.php index e59fc17..4ce717a 100644 --- a/lib/Controller/ReportController.php +++ b/lib/Controller/ReportController.php @@ -163,7 +163,6 @@ class ReportController extends ApiController { * POST /api/v1/reports/{type}/encrypted * Body: { "password": "...", ...params } */ - #[NoCSRFRequired] public function encrypted(string $type): DataDownloadResponse|JSONResponse { try { $password = $this->request->getParam('password'); diff --git a/tests/Unit/CsrfProtectionTest.php b/tests/Unit/CsrfProtectionTest.php new file mode 100644 index 0000000..56ce9fd --- /dev/null +++ b/tests/Unit/CsrfProtectionTest.php @@ -0,0 +1,132 @@ + method names that are POST/DELETE and + * MUST NOT have #[NoCSRFRequired]. + */ + private function getPostDeleteEndpoints(): array { + return [ + // ExportController — encrypted exports use POST + \OCA\Mitgliederverwaltung\Controller\ExportController::class => [ + 'membersEncrypted', + 'feesEncrypted', + 'birthdaysEncrypted', + 'bundleSensitive', + 'bankingEncrypted', + ], + // ReportController — encrypted report uses POST + \OCA\Mitgliederverwaltung\Controller\ReportController::class => [ + 'encrypted', + ], + // DsgvoController — export (POST) and hardDelete (DELETE) + \OCA\Mitgliederverwaltung\Controller\DsgvoController::class => [ + 'export', + 'hardDelete', + ], + ]; + } + + /** + * Map of controller class => method names that are GET and + * MAY have #[NoCSRFRequired] (read-only endpoints). + */ + private function getGetEndpoints(): array { + return [ + \OCA\Mitgliederverwaltung\Controller\ExportController::class => [ + 'members', + 'fees', + 'birthdays', + 'bundle', + 'entityTypes', + 'entity', + ], + \OCA\Mitgliederverwaltung\Controller\ReportController::class => [ + 'index', + 'preview', + 'pdf', + ], + ]; + } + + /** + * @dataProvider postDeleteEndpointsProvider + */ + public function testPostDeleteEndpointsRequireCsrf(string $controllerClass, string $method): void { + $rc = new ReflectionClass($controllerClass); + $rm = $rc->getMethod($method); + + $attrs = $rm->getAttributes(NoCSRFRequired::class); + + $this->assertEmpty( + $attrs, + sprintf( + '%s::%s is a POST/DELETE endpoint but has #[NoCSRFRequired] — ' + . 'this bypasses CSRF protection on a state-changing endpoint.', + $controllerClass, + $method + ) + ); + } + + /** + * @dataProvider getEndpointsProvider + */ + public function testGetEndpointsAllowNoCsrf(string $controllerClass, string $method): void { + $rc = new ReflectionClass($controllerClass); + $rm = $rc->getMethod($method); + + $attrs = $rm->getAttributes(NoCSRFRequired::class); + + $this->assertNotEmpty( + $attrs, + sprintf( + '%s::%s is a GET (read-only) endpoint and should have #[NoCSRFRequired] ' + . 'for compatibility with Nextcloud API calls.', + $controllerClass, + $method + ) + ); + } + + public static function postDeleteEndpointsProvider(): array { + $data = []; + $endpoints = (new self('dummy'))->getPostDeleteEndpoints(); + foreach ($endpoints as $class => $methods) { + foreach ($methods as $method) { + $shortClass = (new ReflectionClass($class))->getShortName(); + $data["$shortClass::$method"] = [$class, $method]; + } + } + return $data; + } + + public static function getEndpointsProvider(): array { + $data = []; + $endpoints = (new self('dummy'))->getGetEndpoints(); + foreach ($endpoints as $class => $methods) { + foreach ($methods as $method) { + $shortClass = (new ReflectionClass($class))->getShortName(); + $data["$shortClass::$method"] = [$class, $method]; + } + } + return $data; + } +}