diff --git a/lib/Controller/AcController.php b/lib/Controller/AcController.php index 432d0f33..dcbb0abf 100644 --- a/lib/Controller/AcController.php +++ b/lib/Controller/AcController.php @@ -27,7 +27,6 @@ namespace OCA\Procest\Controller; use OCA\Procest\Service\ZgwService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IL10N; @@ -52,7 +51,7 @@ * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ -class AcController extends Controller +class AcController extends ZgwController { /** * Constructor. @@ -177,6 +176,20 @@ public function create(): JSONResponse return $authError; } + // C2: Gate writes on ac.aanmaken scope. + if ($this->zgwService->consumerHasScope($this->request, 'ac', 'ac.aanmaken') === false) { + return new JSONResponse( + data: [ + 'type' => 'PermissionDenied', + 'code' => 'permission_denied', + 'title' => 'U heeft geen toestemming om autorisaties aan te maken.', + 'status' => Http::STATUS_FORBIDDEN, + 'detail' => 'Scope ac.aanmaken is vereist.', + ], + statusCode: Http::STATUS_FORBIDDEN + ); + } + if ($this->zgwService->getConsumerMapper() === null) { return new JSONResponse( data: ['detail' => 'Consumer service not available'], @@ -187,6 +200,22 @@ public function create(): JSONResponse try { $body = $this->zgwService->getRequestBody($this->request); + // C2: Block setting superuser=true unless the requesting consumer is itself a superuser. + if (($body['heeftAlleAutorisaties'] ?? false) === true + && $this->zgwService->consumerHasScope($this->request, 'ac', 'ac.superuser') === false + ) { + return new JSONResponse( + data: [ + 'invalidParams' => [[ + 'name' => 'heeftAlleAutorisaties', + 'code' => 'permission_denied', + 'reason' => 'Alleen superuser-consumers mogen heeftAlleAutorisaties op true zetten.', + ]], + ], + statusCode: Http::STATUS_FORBIDDEN + ); + } + // Run AC business rules validation. $validationError = $this->validateApplicatieBody(body: $body); if ($validationError !== null) { @@ -288,6 +317,20 @@ public function update(string $uuid): JSONResponse return $authError; } + // C2: Gate writes on ac.bijwerken scope. + if ($this->zgwService->consumerHasScope($this->request, 'ac', 'ac.bijwerken') === false) { + return new JSONResponse( + data: [ + 'type' => 'PermissionDenied', + 'code' => 'permission_denied', + 'title' => 'U heeft geen toestemming om autorisaties bij te werken.', + 'status' => Http::STATUS_FORBIDDEN, + 'detail' => 'Scope ac.bijwerken is vereist.', + ], + statusCode: Http::STATUS_FORBIDDEN + ); + } + if ($this->zgwService->getConsumerMapper() === null) { return new JSONResponse( data: ['detail' => 'Consumer service not available'], @@ -369,6 +412,20 @@ public function destroy(string $uuid): JSONResponse return $authError; } + // C2: Gate writes on ac.verwijderen scope. + if ($this->zgwService->consumerHasScope($this->request, 'ac', 'ac.verwijderen') === false) { + return new JSONResponse( + data: [ + 'type' => 'PermissionDenied', + 'code' => 'permission_denied', + 'title' => 'U heeft geen toestemming om autorisaties te verwijderen.', + 'status' => Http::STATUS_FORBIDDEN, + 'detail' => 'Scope ac.verwijderen is vereist.', + ], + statusCode: Http::STATUS_FORBIDDEN + ); + } + if ($this->zgwService->getConsumerMapper() === null) { return new JSONResponse( data: ['detail' => 'Consumer service not available'], diff --git a/lib/Controller/BrcController.php b/lib/Controller/BrcController.php index 354d1f3f..d885cb6c 100644 --- a/lib/Controller/BrcController.php +++ b/lib/Controller/BrcController.php @@ -31,7 +31,6 @@ use OCA\Procest\Service\SettingsService; use OCA\Procest\Service\ZgwService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; @@ -56,7 +55,7 @@ * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ -class BrcController extends Controller +class BrcController extends ZgwController { /** * The ZGW API identifier for the Besluiten register. diff --git a/lib/Controller/DrcController.php b/lib/Controller/DrcController.php index 67ad87f2..58c0b922 100644 --- a/lib/Controller/DrcController.php +++ b/lib/Controller/DrcController.php @@ -30,7 +30,6 @@ namespace OCA\Procest\Controller; use OCA\Procest\Service\ZgwService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\JSONResponse; @@ -54,7 +53,7 @@ * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ -class DrcController extends Controller +class DrcController extends ZgwController { /** * The ZGW API identifier for the Documenten register. diff --git a/lib/Controller/NrcController.php b/lib/Controller/NrcController.php index f207c677..5961eee6 100644 --- a/lib/Controller/NrcController.php +++ b/lib/Controller/NrcController.php @@ -30,7 +30,6 @@ namespace OCA\Procest\Controller; use OCA\Procest\Service\ZgwService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; @@ -45,7 +44,7 @@ * * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ -class NrcController extends Controller +class NrcController extends ZgwController { /** * The ZGW API identifier for the Notificaties register. diff --git a/lib/Controller/ZgwController.php b/lib/Controller/ZgwController.php new file mode 100644 index 00000000..344f0906 --- /dev/null +++ b/lib/Controller/ZgwController.php @@ -0,0 +1,41 @@ + + * @copyright 2024 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-License-Identifier: EUPL-1.2 + * SPDX-FileCopyrightText: 2024 Conduction B.V. + * + * @version GIT: + * + * @link https://procest.nl + */ + +declare(strict_types=1); + +namespace OCA\Procest\Controller; + +use OCP\AppFramework\Controller; + +/** + * Abstract base class for all ZGW API controllers. + * + * ZgwAuthMiddleware uses `instanceof ZgwController` to identify which + * controllers fall under ZGW JWT authentication and scope enforcement. + * Any controller that handles a ZGW API endpoint must extend this class + * so the middleware's guard is actually exercised. + */ +abstract class ZgwController extends Controller +{ +}//end class diff --git a/lib/Controller/ZrcController.php b/lib/Controller/ZrcController.php index b2fe2079..81d1992d 100644 --- a/lib/Controller/ZrcController.php +++ b/lib/Controller/ZrcController.php @@ -35,7 +35,6 @@ use DateInterval; use DateTime; use OCA\Procest\Service\ZgwService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IL10N; @@ -58,7 +57,7 @@ * @SuppressWarnings(PHPMD.TooManyMethods) * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ -class ZrcController extends Controller +class ZrcController extends ZgwController { /** * The ZGW API group for this controller. diff --git a/lib/Controller/ZtcController.php b/lib/Controller/ZtcController.php index fb422eff..0a7a15ff 100644 --- a/lib/Controller/ZtcController.php +++ b/lib/Controller/ZtcController.php @@ -31,7 +31,6 @@ namespace OCA\Procest\Controller; use OCA\Procest\Service\ZgwService; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; @@ -51,7 +50,7 @@ * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ -class ZtcController extends Controller +class ZtcController extends ZgwController { /** * The ZGW API identifier for the Catalogi register. diff --git a/lib/Middleware/ZgwAuthMiddleware.php b/lib/Middleware/ZgwAuthMiddleware.php index de773714..f8b2f822 100644 --- a/lib/Middleware/ZgwAuthMiddleware.php +++ b/lib/Middleware/ZgwAuthMiddleware.php @@ -177,6 +177,7 @@ public function beforeController($controller, $methodName): void } // Validate JWT signature via OpenRegister's AuthorizationService. + // M3: Log detailed message server-side; surface only a generic message to caller. try { $this->authorizationService->authorizeJwt(authorization: $authorization); } catch (\Exception $e) { @@ -184,7 +185,7 @@ public function beforeController($controller, $methodName): void 'ZGW auth failed: '.$e->getMessage() ); throw new ZgwAuthException( - message: $e->getMessage(), + message: 'Authenticatiegegevens zijn niet geldig.', statusCode: Http::STATUS_FORBIDDEN ); } diff --git a/lib/Service/ZgwService.php b/lib/Service/ZgwService.php index 15756ae4..a30ef5b3 100644 --- a/lib/Service/ZgwService.php +++ b/lib/Service/ZgwService.php @@ -622,13 +622,20 @@ public function validateJwtAuth(IRequest $request): ?JSONResponse authorization: $authHeader ); } catch (\Throwable $e) { + // M3: Log detail server-side but never surface internal JWT validation + // messages in the HTTP response — they aid algorithm/issuer enumeration. + $this->logger->warning( + 'ZGW JWT validation failed: '.$e->getMessage(), + ['app' => 'procest'] + ); + return new JSONResponse( data: [ 'type' => 'NotAuthenticated', 'code' => 'not_authenticated', 'title' => 'Authenticatiegegevens zijn niet geldig.', 'status' => 403, - 'detail' => $e->getMessage(), + 'detail' => 'Authenticatiegegevens zijn niet geldig.', ], statusCode: Http::STATUS_FORBIDDEN ); @@ -653,29 +660,34 @@ public function validateJwtAuth(IRequest $request): ?JSONResponse */ public function consumerHasScope(IRequest $request, string $component, string $scope): bool { + // H2: Fail closed — if ConsumerMapper is not available we cannot verify scope, + // so we must deny rather than grant. if ($this->consumerMapper === null) { - return true; + return false; } try { $authHeader = $request->getHeader('Authorization'); $token = str_replace('Bearer ', '', $authHeader); $parts = explode('.', $token); + // H2: Malformed JWT → deny, not grant. if (count($parts) !== 3) { - return true; + return false; } $payload = json_decode(base64_decode($parts[1]), true); $clientId = $payload['client_id'] ?? ($payload['iss'] ?? null); + // H2: Missing client_id/iss → deny, not grant. if ($clientId === null) { - return true; + return false; } $consumers = $this->consumerMapper->findAll( filters: ['name' => $clientId] ); + // H2: No matching consumer → deny, not grant. if (empty($consumers) === true) { - return true; + return false; } $consumer = $consumers[0]; @@ -726,29 +738,33 @@ public function consumerHasScope(IRequest $request, string $component, string $s */ public function getConsumerAuthorisaties(IRequest $request, string $component): ?array { + // H2: Fail closed — if ConsumerMapper unavailable, return empty set (not null/unrestricted). if ($this->consumerMapper === null) { - return null; + return []; } try { $authHeader = $request->getHeader('Authorization'); $token = str_replace('Bearer ', '', $authHeader); $parts = explode('.', $token); + // H2: Malformed JWT → return empty (restricted), not null (unrestricted). if (count($parts) !== 3) { - return null; + return []; } $payload = json_decode(base64_decode($parts[1]), true); $clientId = $payload['client_id'] ?? ($payload['iss'] ?? null); + // H2: Missing client_id/iss → return empty (restricted). if ($clientId === null) { - return null; + return []; } $consumers = $this->consumerMapper->findAll( filters: ['name' => $clientId] ); + // H2: No matching consumer → return empty (restricted). if (empty($consumers) === true) { - return null; + return []; } $consumer = $consumers[0]; diff --git a/tests/Unit/Middleware/ZgwAuthMiddlewareTest.php b/tests/Unit/Middleware/ZgwAuthMiddlewareTest.php index e10abf15..c1e80a69 100644 --- a/tests/Unit/Middleware/ZgwAuthMiddlewareTest.php +++ b/tests/Unit/Middleware/ZgwAuthMiddlewareTest.php @@ -147,6 +147,54 @@ public function testBeforeControllerSkipsNonZgwController(): void }//end testBeforeControllerSkipsNonZgwController() + /** + * Test that ZgwController is abstract and cannot be instantiated directly. + * + * C3: verifies the marker class exists and is abstract so concrete ZGW + * controllers can extend it and the instanceof check in the middleware fires. + * + * @return void + */ + public function testZgwControllerIsAbstract(): void + { + $reflection = new \ReflectionClass(\OCA\Procest\Controller\ZgwController::class); + $this->assertTrue( + $reflection->isAbstract(), + 'ZgwController must be abstract — it is an instanceof marker only' + ); + + }//end testZgwControllerIsAbstract() + + + /** + * Test that concrete ZGW controllers extend ZgwController. + * + * C3: The middleware fires only for instanceof ZgwController. Verifying + * the inheritance chain ensures no ZGW endpoint can accidentally bypass it. + * + * @return void + */ + public function testZgwControllersExtendBase(): void + { + $zgwControllers = [ + \OCA\Procest\Controller\AcController::class, + \OCA\Procest\Controller\ZrcController::class, + \OCA\Procest\Controller\ZtcController::class, + \OCA\Procest\Controller\DrcController::class, + \OCA\Procest\Controller\BrcController::class, + \OCA\Procest\Controller\NrcController::class, + ]; + + foreach ($zgwControllers as $class) { + $this->assertTrue( + is_subclass_of($class, \OCA\Procest\Controller\ZgwController::class), + "$class must extend ZgwController so middleware applies" + ); + } + + }//end testZgwControllersExtendBase() + + /** * Test that afterException returns null for non-ZgwAuthException. *