diff --git a/composer.json b/composer.json index ae1d641..631f996 100644 --- a/composer.json +++ b/composer.json @@ -37,6 +37,7 @@ "phpnomad/tests": "^0.3.0" }, "require": { + "php": ">=8.0", "phpnomad/auth": "^1.0", "phpnomad/asset": "^1.0", "phpnomad/db": "^2.0", diff --git a/lib/Strategies/RestStrategy.php b/lib/Strategies/RestStrategy.php index c711f87..2cd2f76 100644 --- a/lib/Strategies/RestStrategy.php +++ b/lib/Strategies/RestStrategy.php @@ -17,6 +17,8 @@ use PHPNomad\Http\Interfaces\Response; use PHPNomad\Rest\Interfaces\RestStrategy as CoreRestStrategy; use PHPNomad\Utils\Helpers\Arr; +use WeakMap; +use WP_Error; use WP_REST_Request; use WP_REST_Response; @@ -31,12 +33,84 @@ class RestStrategy implements CoreRestStrategy protected CurrentUserResolverStrategy $currentUserResolver; protected LoggerStrategy $logger; + /** + * Per-WP_REST_Request memoization so the permission callback and the + * route callback operate on the SAME framework request instance, and + * the middleware chain runs exactly once per request (middleware may + * mutate request state or perform lookups). + * + * @var WeakMap + */ + private WeakMap $resolvedRequests; + + /** + * Requests whose middleware chain already ran in the permission + * callback, mapped to the middleware outcome (true, or the caught + * non-auth RestException to be re-thrown by the route callback so + * clients keep the rich legacy error shape). + * + * @var WeakMap + */ + private WeakMap $middlewareOutcomes; + public function __construct(HasRestNamespace $restNamespaceProvider, Response $response, CurrentUserResolverStrategy $currentUserResolverStrategy, LoggerStrategy $loggerStrategy) { $this->logger = $loggerStrategy; $this->restNamespaceProvider = $restNamespaceProvider; $this->response = $response; $this->currentUserResolver = $currentUserResolverStrategy; + $this->resolvedRequests = new WeakMap(); + $this->middlewareOutcomes = new WeakMap(); + } + + private function resolveRequest(WP_REST_Request $wpRequest): Request + { + if (!isset($this->resolvedRequests[$wpRequest])) { + $this->resolvedRequests[$wpRequest] = new WordPressRequest($wpRequest, $this->currentUserResolver->getCurrentUser()); + } + + return $this->resolvedRequests[$wpRequest]; + } + + /** + * WordPress-level permission gate derived from the controller's own + * middleware — no controller contract change. The chain runs here, + * once; authorization rejections (401/403) surface as WP_Error so + * WordPress treats the route as protected (and stops advertising it + * as public). Non-auth middleware failures (validation, existence) + * are stored and re-thrown by the route callback, preserving the + * legacy error payload shape clients rely on. + * + * @return true|WP_Error + */ + private function checkPermissions(Controller $controller, WP_REST_Request $wpRequest) + { + if (!$controller instanceof HasMiddleware) { + return true; + } + + $request = $this->resolveRequest($wpRequest); + + try { + Arr::each($controller->getMiddleware($request), fn(Middleware $middleware) => $middleware->process($request)); + $this->middlewareOutcomes[$wpRequest] = true; + } catch (RestException $e) { + if (in_array($e->getCode(), [401, 403], true)) { + return new WP_Error( + $e->getCode() === 401 ? 'rest_not_logged_in' : 'rest_forbidden', + $e->getMessage(), + ['status' => $e->getCode()] + ); + } + + $this->middlewareOutcomes[$wpRequest] = $e; + } catch (Exception $e) { + $this->logger->logException($e); + + return new WP_Error('rest_unexpected_error', 'An unexpected error occurred.', ['status' => 500]); + } + + return true; } /** @@ -45,10 +119,18 @@ public function __construct(HasRestNamespace $restNamespaceProvider, Response $r * @return WP_REST_Response * @throws RestException */ - private function wrapCallback(Controller $controller, Request $request): Response + private function wrapCallback(Controller $controller, Request $request, ?WP_REST_Request $wpRequest = null): Response { - // Maybe process middleware. - if ($controller instanceof HasMiddleware) { + $outcome = $wpRequest !== null ? ($this->middlewareOutcomes[$wpRequest] ?? null) : null; + + if ($outcome instanceof RestException) { + // Middleware already ran in the permission callback and failed + // with a non-auth error — surface it through the legacy formatter. + throw $outcome; + } + + // Run middleware only if the permission callback didn't already. + if ($outcome !== true && $controller instanceof HasMiddleware) { Arr::each($controller->getMiddleware($request), fn(Middleware $middleware) => $middleware->process($request)); } @@ -74,10 +156,10 @@ private function convertEndpointFormat(string $input): string return preg_replace('/\{([\w-]+)}/', '(?P<$1>[\w-]+)', $input); } - protected function handleRequest(Controller $controller, Request $request) + protected function handleRequest(Controller $controller, Request $request, ?WP_REST_Request $wpRequest = null) { try { - $response = $this->wrapCallback($controller, $request); + $response = $this->wrapCallback($controller, $request, $wpRequest); } catch (RestException $e) { $response = $this->response->setStatus($e->getCode())->setJson( [ @@ -113,8 +195,8 @@ public function registerRoute(callable $controllerGetter) $this->convertEndpointFormat($controller->getEndpoint()), [ 'methods' => $controller->getMethod(), - 'callback' => fn(WP_REST_Request $request) => $this->handleRequest($controller, new WordPressRequest($request, $this->currentUserResolver->getCurrentUser())), - 'permission_callback' => '__return_true' + 'callback' => fn(WP_REST_Request $request) => $this->handleRequest($controller, $this->resolveRequest($request), $request), + 'permission_callback' => fn(WP_REST_Request $request) => $this->checkPermissions($controller, $request), ] ); }); diff --git a/tests/Unit/Strategies/RestStrategyPermissionsTest.php b/tests/Unit/Strategies/RestStrategyPermissionsTest.php new file mode 100644 index 0000000..8739d18 --- /dev/null +++ b/tests/Unit/Strategies/RestStrategyPermissionsTest.php @@ -0,0 +1,141 @@ +createMock(HasRestNamespace::class), + $this->createMock(Response::class), + $this->createMock(CurrentUserResolverStrategy::class), + $this->createMock(LoggerStrategy::class) + ); + } + + private function checkPermissions(RestStrategy $strategy, Controller $controller, WP_REST_Request $request) + { + $method = new ReflectionMethod($strategy, 'checkPermissions'); + + return $method->invoke($strategy, $controller, $request); + } + + private function makeControllerWithMiddleware(Middleware $middleware): Controller + { + return new class($middleware) implements Controller, HasMiddleware { + public function __construct(private Middleware $middleware) + { + } + + public function getEndpoint(): string + { + return '/test'; + } + + public function getMethod(): string + { + return 'GET'; + } + + public function getResponse(Request $request): Response + { + throw new Exception('not used'); + } + + public function getMiddleware(Request $request): array + { + return [$this->middleware]; + } + }; + } + + public function testControllerWithoutMiddlewareIsPublic(): void + { + $controller = $this->createMock(Controller::class); + + $result = $this->checkPermissions($this->makeStrategy(), $controller, new WP_REST_Request()); + + $this->assertTrue($result); + } + + public function testUnauthorizedMiddlewareRejectionBecomesWpError(): void + { + $middleware = new class implements Middleware { + public function process(Request $request): void + { + throw new RestException('You are not allowed to do that.', [], 403); + } + }; + + $result = $this->checkPermissions($this->makeStrategy(), $this->makeControllerWithMiddleware($middleware), new WP_REST_Request()); + + $this->assertInstanceOf(WP_Error::class, $result); + } + + public function testNonAuthMiddlewareFailurePassesPermissionGate(): void + { + $middleware = new class implements Middleware { + public function process(Request $request): void + { + throw new RestException('Validation failed.', [], 422); + } + }; + + $result = $this->checkPermissions($this->makeStrategy(), $this->makeControllerWithMiddleware($middleware), new WP_REST_Request()); + + $this->assertTrue($result); + } + + public function testMiddlewareRunsExactlyOnceAcrossPermissionAndCallback(): void + { + $middleware = new class implements Middleware { + public int $runs = 0; + + public function process(Request $request): void + { + $this->runs++; + } + }; + + $controller = $this->makeControllerWithMiddleware($middleware); + $strategy = $this->makeStrategy(); + $wpRequest = new WP_REST_Request(); + + $this->checkPermissions($strategy, $controller, $wpRequest); + + $wrap = new ReflectionMethod($strategy, 'wrapCallback'); + $resolve = new ReflectionMethod($strategy, 'resolveRequest'); + + try { + $wrap->invoke($strategy, $controller, $resolve->invoke($strategy, $wpRequest), $wpRequest); + } catch (Exception $e) { + // getResponse() intentionally throws; middleware count is the assertion target. + } + + $this->assertSame(1, $middleware->runs); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 13c69d8..d8f6a96 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -104,3 +104,35 @@ function determine_locale(): string return $_wp_determined_locale ?? 'en_US'; } } + +if (!class_exists('WP_Error')) { + class WP_Error + { + public function __construct( + public string $code = '', + public string $message = '', + public array $data = [] + ) { + } + } +} + +if (!class_exists('WP_REST_Request')) { + class WP_REST_Request + { + public function get_params(): array + { + return []; + } + + public function get_param(string $key) + { + return null; + } + + public function get_headers(): array + { + return []; + } + } +}