From 41967a296609e86dab5c9a5947ebfe134e827c2e Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Sun, 25 Oct 2026 18:06:53 +0600 Subject: [PATCH 1/3] feat(router): per-route and group REST permission callbacks APIRouter registered every REST route with permission_callback => '__return_true', leaving middleware as the only gate. WordPress.org reviewers reject that for privileged routes, and WordPress cannot answer 401/403 before the callback runs. RouteBase::permission() sets a callback for a whole group and RouteRegister::permission() overrides it per route. Routes without one keep '__return_true', so existing consumers are unchanged. Assisted-By: AI --- src/Http/Router/APIRouter.php | 2 +- src/Http/Router/Route.php | 2 ++ src/Http/Router/RouteBase.php | 29 ++++++++++++++++++ src/Http/Router/RouteRegister.php | 27 +++++++++++++++++ tests/Router/TransportRegistrationTest.php | 35 ++++++++++++++++++++++ 5 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/Http/Router/APIRouter.php b/src/Http/Router/APIRouter.php index 672d3ef..85bb8f0 100644 --- a/src/Http/Router/APIRouter.php +++ b/src/Http/Router/APIRouter.php @@ -38,7 +38,7 @@ public function addRoute(RouteRegister $route): void $args[] = [ 'methods' => $this->getMethod($method), 'callback' => [$route, 'handleRequest'], - 'permission_callback' => '__return_true', + 'permission_callback' => $route->getPermissionCallback(), ]; } diff --git a/src/Http/Router/Route.php b/src/Http/Router/Route.php index 22dce1b..699f3ae 100644 --- a/src/Http/Router/Route.php +++ b/src/Http/Router/Route.php @@ -12,6 +12,8 @@ * @method static RouteBase isTokenIgnored() * @method static RouteBase ignoreToken() * @method static RouteBase getMiddleware() + * @method static RouteBase permission(callable $callback) + * @method static RouteBase getPermission() * @method static RouteBase getRoutePrefix() * @method static RouteBase group(Closure $callback) * @method static RouteBase getRouter() diff --git a/src/Http/Router/RouteBase.php b/src/Http/Router/RouteBase.php index 5cc0c88..e9e12fc 100644 --- a/src/Http/Router/RouteBase.php +++ b/src/Http/Router/RouteBase.php @@ -45,6 +45,13 @@ final class RouteBase private array $_middleware = []; + /** + * REST permission callback shared by every route in this group. + * + * @var null|callable + */ + private $_permission; + private static ?self $_isGrouped = null; /** @@ -152,6 +159,28 @@ public function getMiddleware(): array return $this->_middleware; } + /** + * Sets the REST permission callback for every route in this group. + * + * @param callable $callback receives the WP_REST_Request; return true, false or WP_Error + * + * @return RouteBase + */ + public function permission(callable $callback): self + { + $this->_permission = $callback; + + return $this; + } + + /** + * Returns the group's REST permission callback, if any. + */ + public function getPermission(): ?callable + { + return $this->_permission; + } + /** * Return prefix for request. * diff --git a/src/Http/Router/RouteRegister.php b/src/Http/Router/RouteRegister.php index 4ba53a2..b9018e9 100644 --- a/src/Http/Router/RouteRegister.php +++ b/src/Http/Router/RouteRegister.php @@ -29,6 +29,13 @@ final class RouteRegister private array $_middleware = []; + /** + * REST permission callback for this route; falls back to the group's. + * + * @var null|callable + */ + private $_permission; + /** * Instance of rest request. * @@ -156,6 +163,26 @@ public function middleware(): self return $this; } + /** + * Sets the REST permission callback for this route. + * + * @param callable $callback receives the WP_REST_Request; return true, false or WP_Error + */ + public function permission(callable $callback): self + { + $this->_permission = $callback; + + return $this; + } + + /** + * REST permission callback: the route's own, else its group's, else public. + */ + public function getPermissionCallback(): callable + { + return $this->_permission ?? $this->_routeBase->getPermission() ?? '__return_true'; + } + public function handleMiddleware(): bool { try { diff --git a/tests/Router/TransportRegistrationTest.php b/tests/Router/TransportRegistrationTest.php index 354096c..5b37ab5 100644 --- a/tests/Router/TransportRegistrationTest.php +++ b/tests/Router/TransportRegistrationTest.php @@ -40,6 +40,41 @@ public function testRESTTransportRouteRegistrationPreservesNamespaceVersionPathA assertTest(\is_callable($registration['args'][0]['permission_callback']), 'REST permission callback is not callable'); } + public function testRESTTransportDefaultsToAPublicPermissionCallback(): void + { + $route = (new RouteBase())->get('entries', fn () => 'ok'); + + (new APIRouter(new Router('api', 'example', 'v1')))->addRoute($route); + + assertSameValue('__return_true', WpKitTestState::$restRoutes[0]['args'][0]['permission_callback'], 'default permission changed'); + } + + public function testRESTTransportUsesTheGroupPermissionCallback(): void + { + $permission = fn () => false; + $group = (new RouteBase())->permission($permission); + $route = $group->match('GET,POST', 'entries', fn () => 'ok'); + + (new APIRouter(new Router('api', 'example', 'v1')))->addRoute($route); + + foreach (WpKitTestState::$restRoutes[0]['args'] as $args) { + assertSameValue($permission, $args['permission_callback'], 'group permission not applied to every method'); + } + } + + public function testRESTTransportRoutePermissionOverridesTheGroup(): void + { + $routePermission = fn () => true; + $route = (new RouteBase()) + ->permission(fn () => false) + ->get('entries', fn () => 'ok') + ->permission($routePermission); + + (new APIRouter(new Router('api', 'example', 'v1')))->addRoute($route); + + assertSameValue($routePermission, WpKitTestState::$restRoutes[0]['args'][0]['permission_callback'], 'route permission did not win'); + } + public function testRESTTransportWordPressMethodConstantsRemainMapped(): void { $router = new Router('api', 'example', 'v1'); From ade45c09f761a2ab0d16ed2902a6f358dc660cb7 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Mon, 26 Oct 2026 15:10:39 +0600 Subject: [PATCH 2/3] test(router): cover group inheritance for nested groups and inner routes Route attributes declared inside a group (permission, middleware) were dropped, nested groups ignored their parent and reset the outer group on close, and a throwing group callback leaked its state. Assisted-By: AI --- tests/Router/RouteDefinitionTest.php | 52 ++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/Router/RouteDefinitionTest.php b/tests/Router/RouteDefinitionTest.php index 7659829..0475a1c 100644 --- a/tests/Router/RouteDefinitionTest.php +++ b/tests/Router/RouteDefinitionTest.php @@ -97,6 +97,58 @@ public function testRouteDefinitionGroupsApplySharedPrefixAndMiddleware(): void assertSameValue(['auth'], $route->getMiddleware(), 'group middleware was not inherited'); } + public function testRouteDefinitionRouteAttributesInsideAGroupAreKept(): void + { + $router = new Router('api', 'example', 'v1'); + $permission = fn () => false; + + Route::prefix('demo')->group(function () use ($permission) { + Route::permission($permission)->middleware('audit')->get('entries', fn () => 'ok'); + }); + + $route = $router->getRoute(0); + assertSameValue('demo', $route->getRoutePrefix(), 'group prefix was lost'); + assertSameValue($permission, $route->getPermissionCallback(), 'route permission inside a group was dropped'); + assertSameValue(['audit'], $route->getMiddleware(), 'route middleware inside a group was dropped'); + } + + public function testRouteDefinitionNestedGroupsInheritFromTheirParent(): void + { + $router = new Router('api', 'example', 'v1'); + $permission = fn () => false; + + Route::permission($permission)->prefix('admin')->middleware('auth')->group(function () { + Route::prefix('inner')->middleware('audit')->group(function () { + Route::get('nested', fn () => 'ok'); + }); + Route::get('sibling', fn () => 'ok'); + }); + + $nested = $router->getRoute(0); + assertSameValue('admin/inner', $nested->getRoutePrefix(), 'nested prefix not joined to the parent'); + assertSameValue($permission, $nested->getPermissionCallback(), 'nested group lost the parent permission'); + assertSameValue(['auth', 'audit'], $nested->getMiddleware(), 'nested group lost the parent middleware'); + + $sibling = $router->getRoute(1); + assertSameValue('admin', $sibling->getRoutePrefix(), 'outer group not restored after a nested group'); + assertSameValue($permission, $sibling->getPermissionCallback(), 'outer permission not restored after a nested group'); + } + + public function testRouteDefinitionGroupStateIsClearedWhenTheCallbackThrows(): void + { + $router = new Router('api', 'example', 'v1'); + + try { + Route::prefix('broken')->group(function () { + throw new \RuntimeException('boom'); + }); + } catch (\RuntimeException) { + } + Route::get('after', fn () => 'ok'); + + assertSameValue(null, $router->getRoute(0)->getRoutePrefix(), 'group leaked past a thrown callback'); + } + public function testRouteDefinitionPlaceholdersExposeRegexMetadata(): void { new Router('ajax', 'example', null); From bba7415bb033634970f1b2d962885d3054217300 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Mon, 26 Oct 2026 15:10:42 +0600 Subject: [PATCH 3/3] fix(router): inherit group attributes through nested groups Each RouteBase now records the group it was declared in and resolves prefix, middleware, permission, noAuth and ignoreToken through that parent at read time. Routes register against their own RouteBase, so attributes set inside a group are kept, and group() restores the enclosing group in a finally block. Nested prefixes now join (admin/inner) instead of the inner one replacing the outer. Assisted-By: AI --- src/Http/Router/RouteBase.php | 48 +++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/src/Http/Router/RouteBase.php b/src/Http/Router/RouteBase.php index e9e12fc..9db914c 100644 --- a/src/Http/Router/RouteBase.php +++ b/src/Http/Router/RouteBase.php @@ -52,8 +52,21 @@ final class RouteBase */ private $_permission; + /** + * Enclosing group this route or group was declared in; attributes inherit from it. + */ + private ?self $_parent; + private static ?self $_isGrouped = null; + /** + * Captures the group being declared, so nested routes and groups inherit from it. + */ + public function __construct() + { + $this->_parent = self::$_isGrouped; + } + /** * Handle static call to route. * @@ -112,7 +125,7 @@ public function noAuth(): self */ public function isNoAuth(): ?bool { - return $this->_noAuth; + return $this->_noAuth ?? $this->_parent?->isNoAuth(); } /** @@ -122,7 +135,7 @@ public function isNoAuth(): ?bool */ public function isTokenIgnored(): ?bool { - return $this->_ignoreToken; + return $this->_ignoreToken ?? $this->_parent?->isTokenIgnored(); } /** @@ -156,7 +169,7 @@ public function middleware(): self */ public function getMiddleware(): array { - return $this->_middleware; + return array_merge($this->_parent?->getMiddleware() ?? [], $this->_middleware); } /** @@ -178,7 +191,7 @@ public function permission(callable $callback): self */ public function getPermission(): ?callable { - return $this->_permission; + return $this->_permission ?? $this->_parent?->getPermission(); } /** @@ -188,7 +201,16 @@ public function getPermission(): ?callable */ public function getRoutePrefix() { - return $this->_prefix; + $parentPrefix = $this->_parent?->getRoutePrefix(); + if ($parentPrefix === null || $parentPrefix === '') { + return $this->_prefix; + } + + if ($this->_prefix === null || $this->_prefix === '') { + return $parentPrefix; + } + + return rtrim($parentPrefix, '/') . '/' . ltrim($this->_prefix, '/'); } /** @@ -200,9 +222,14 @@ public function getRoutePrefix() */ public function group(Closure $callback): self { + $enclosing = self::$_isGrouped; self::$_isGrouped = $this; - $callback(); - self::$_isGrouped = null; + + try { + $callback(); + } finally { + self::$_isGrouped = $enclosing; + } return $this; } @@ -228,11 +255,6 @@ public function getRouter() */ private function getRegistrar(): RouteRegister { - $instance = $this; - if (!\is_null(self::$_isGrouped)) { - $instance = self::$_isGrouped; - } - - return new RouteRegister($instance); + return new RouteRegister($this); } }