fix(api): return a full batch response when one request is forbidden
An AccessDeniedException raised by a single request was relayed out of the batch loop, so the whole call answered with one error object instead of the array of responses required by the JSON-RPC spec. Authentication failures still fail the whole call. Fixes #3879
This commit is contained in:
1 parent
f06233d996
commit
16430fd3d2
2 files changed
+76
-6
No files matched your search
@@ -2,6 +2,9 @@
|
||||
|
||||
namespace JsonRPC\Request;
|
||||
|
||||
use JsonRPC\Exception\AccessDeniedException;
|
||||
use JsonRPC\Response\ResponseBuilder;
|
||||
|
||||
/**
|
||||
* Class BatchRequestParser
|
||||
*
|
||||
@@ -21,12 +24,21 @@ class BatchRequestParser extends RequestParser
|
||||
$responses = array();
|
||||
|
||||
foreach ($this->payload as $payload) {
|
||||
$responses[] = RequestParser::create()
|
||||
->withPayload($payload)
|
||||
->withProcedureHandler($this->procedureHandler)
|
||||
->withMiddlewareHandler($this->middlewareHandler)
|
||||
->withLocalException($this->localExceptions)
|
||||
->parse();
|
||||
try {
|
||||
$responses[] = RequestParser::create()
|
||||
->withPayload($payload)
|
||||
->withProcedureHandler($this->procedureHandler)
|
||||
->withMiddlewareHandler($this->middlewareHandler)
|
||||
->withLocalException($this->localExceptions)
|
||||
->parse();
|
||||
} catch (AccessDeniedException $e) {
|
||||
//Bad credentials still fail the whole call, but a forbidden
|
||||
//resource concerns only the request that asked for it
|
||||
$responses[] = ResponseBuilder::create()
|
||||
->withId(isset($payload['id']) ? $payload['id'] : null)
|
||||
->withException($e)
|
||||
->build();
|
||||
}
|
||||
}
|
||||
|
||||
$responses = array_filter($responses);
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
<?php
|
||||
|
||||
namespace KanboardTests\units\Api;
|
||||
|
||||
use JsonRPC\Exception\AccessDeniedException;
|
||||
use JsonRPC\Exception\AuthenticationFailureException;
|
||||
use JsonRPC\Server;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
class BatchRequestTest extends TestCase
|
||||
{
|
||||
public function testForbiddenProcedureOnlyFailsItsOwnRequest()
|
||||
{
|
||||
$server = new Server('[
|
||||
{"jsonrpc": "2.0", "method": "getVersion", "id": 1},
|
||||
{"jsonrpc": "2.0", "method": "getProjectById", "id": 2},
|
||||
{"jsonrpc": "2.0", "method": "getVersion", "id": 3}
|
||||
]');
|
||||
|
||||
$server->register('getVersion', function () {
|
||||
return '1.2.3';
|
||||
});
|
||||
|
||||
$server->register('getProjectById', function () {
|
||||
throw new AccessDeniedException('Forbidden');
|
||||
});
|
||||
|
||||
$this->assertEquals(
|
||||
json_decode('[
|
||||
{"jsonrpc": "2.0", "result": "1.2.3", "id": 1},
|
||||
{"jsonrpc": "2.0", "error": {"code": 403, "message": "Forbidden"}, "id": 2},
|
||||
{"jsonrpc": "2.0", "result": "1.2.3", "id": 3}
|
||||
]', true),
|
||||
json_decode($server->execute(), true)
|
||||
);
|
||||
}
|
||||
|
||||
public function testWrongCredentialsStillFailTheWholeBatch()
|
||||
{
|
||||
$server = new Server('[
|
||||
{"jsonrpc": "2.0", "method": "getVersion", "id": 1},
|
||||
{"jsonrpc": "2.0", "method": "getProjectById", "id": 2}
|
||||
]');
|
||||
|
||||
$server->register('getVersion', function () {
|
||||
return '1.2.3';
|
||||
});
|
||||
|
||||
$server->register('getProjectById', function () {
|
||||
throw new AuthenticationFailureException('Wrong credentials');
|
||||
});
|
||||
|
||||
$this->assertEquals(
|
||||
json_decode('{"jsonrpc": "2.0", "error": {"code": 401, "message": "Unauthorized"}, "id": null}', true),
|
||||
json_decode($server->execute(), true)
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user