Skip to content

Commit fcdaf3e

Browse files
committed
fix: Return false in permission callback blocks user
1 parent 2d5ca48 commit fcdaf3e

File tree

4 files changed

+36
-9
lines changed

4 files changed

+36
-9
lines changed

src/Endpoint.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,8 +312,8 @@ public function permissionCallback(WP_REST_Request $request): bool|WP_Error
312312
'endpoint' => $this,
313313
'request' => $request,
314314
];
315-
$result = $this->runHandlers($this->permissionHandlers, $dependencies);
316-
if (is_wp_error($result)) {
315+
$result = $this->runHandlers($this->permissionHandlers, $dependencies, isToReturnResult: true, isPermissionCallback: true);
316+
if (is_wp_error($result) || $result === false) {
317317
return $result;
318318
}
319319

@@ -371,7 +371,7 @@ protected function replaceSpecialValue(WP_REST_Request $request, string $value):
371371
*
372372
* @throws InvocationException
373373
*/
374-
protected function runHandlers(array $allHandlers, array $dependencies, bool $isToReturnResult = false): mixed
374+
protected function runHandlers(array $allHandlers, array $dependencies, bool $isToReturnResult = false, bool $isPermissionCallback = false): mixed
375375
{
376376
$result = null;
377377
foreach ($allHandlers as $handler) {
@@ -388,6 +388,10 @@ protected function runHandlers(array $allHandlers, array $dependencies, bool $is
388388
if (is_wp_error($result) || $result instanceof WP_REST_Response) {
389389
return $result;
390390
}
391+
392+
if ($isPermissionCallback && $result === false) {
393+
return $result;
394+
}
391395
}
392396

393397
return $isToReturnResult ? $result : null;

tests/Integration/AppMultipleRoutersTest.php

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
'/my-api/v1/my-actions/v2/middleware/on-request/(?P<action>\w+)',
6464
'/my-api/v1/my-actions/v2/middleware/on-response/(?P<action>\w+)',
6565
'/my-api/v1/my-actions/v2/permission/(?P<action>\w+)',
66+
'/my-api/v1/my-actions/v2/bool/permission/(?P<action>\w+)',
6667
])
6768
->and($routes['/my-api/v1/my-posts/v1/(?P<ID>[\\d]+)'])
6869
->toBeArray()
@@ -75,6 +76,9 @@
7576
->toHaveCount(1)
7677
->and($routes['/my-api/v1/my-actions/v2/permission/(?P<action>\w+)'])
7778
->toBeArray()
79+
->toHaveCount(1)
80+
->and($routes['/my-api/v1/my-actions/v2/bool/permission/(?P<action>\w+)'])
81+
->toBeArray()
7882
->toHaveCount(1);
7983
})->group('multiple');
8084

@@ -134,7 +138,7 @@
134138
expect($response->get_status())->toBe(200);
135139
})->with(['on-request', 'on-response'])->group('multiple');
136140

137-
test('Trigger no permissions in permission callback', function () {
141+
test('Trigger no permissions in permission callback - WP Error', function () {
138142
$request = new WP_REST_Request('GET', '/my-api/v1/my-actions/v2/permission/notAllowed');
139143
$response = $this->server->dispatch($request);
140144
expect($response->get_status())->toBe(403)
@@ -144,8 +148,20 @@
144148
->toHaveProperty('data', ['status' => 403]);
145149
})->group('multiple');
146150

147-
test('Trigger has permissions in permission callback', function () {
148-
$request = new WP_REST_Request('GET', '/my-api/v1/my-actions/v2/permission/allowed');
151+
test('Trigger no permissions in permission callback - boolean', function () {
152+
$request = new WP_REST_Request('GET', '/my-api/v1/my-actions/v2/bool/permission/notAllowed');
149153
$response = $this->server->dispatch($request);
150-
expect($response->get_status())->toBe(200);
154+
expect($response->get_status())->toBe(401)
155+
->and((object) $response->get_data())
156+
->toHaveProperty('code', 'rest_forbidden')
157+
->toHaveProperty('message', 'Sorry, you are not allowed to do that.')
158+
->toHaveProperty('data', ['status' => 401]);
151159
})->group('multiple');
160+
161+
test('Trigger has permissions in permission callback', function (string $endpoint) {
162+
$request = new WP_REST_Request('GET', $endpoint);
163+
$response = $this->server->dispatch($request);
164+
expect($response->get_status())->toBe(200);
165+
})
166+
->with(['/my-api/v1/my-actions/v2/permission/allowed', '/my-api/v1/my-actions/v2/bool/permission/allowed'])
167+
->group('multiple');

tests/Integration/Routers/ActionsRouter.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,9 @@
4343
})
4444
->permission($triggerPermissionCallback);
4545

46+
$router->get('/bool/permission/(?P<action>\w+)', function (): bool {
47+
return true;
48+
})
49+
->permission(fn (string $action) => $action === 'allowed');
50+
4651
return $router;

tests/Unit/EndpointTest.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,11 +342,13 @@ public function hey(): void {}
342342
Helpers::setNonPublicClassProperty($mockedEndpoint, 'permissionHandlers', ['test-permission-handler']);
343343
$mockedEndpoint->shouldReceive('runHandlers')
344344
->once()
345-
->with(['test-permission-handler'], Mockery::type('array'))
345+
->with(['test-permission-handler'], Mockery::type('array'), true, true)
346346
->andReturn($returnValue);
347347
expect($mockedEndpoint->permissionCallback($req))
348348
->toBe($returnValue ?? true);
349-
})->with([null, WpError::class])->group('endpoint', 'permission', 'permissionCallback');
349+
})
350+
->with([null, WpError::class, false, true])
351+
->group('endpoint', 'permission', 'permissionCallback');
350352

351353
// callback
352354

0 commit comments

Comments
 (0)