Skip to content

Commit

Permalink
Fix validation of REST array parameters.
Browse files Browse the repository at this point in the history
  • Loading branch information
@aaronweeden
aaronweeden committed Dec 7, 2023
1 parent d9ed03e commit d4dedd6
Show file tree
Hide file tree
Showing 9 changed files with 592 additions and 38 deletions.
15 changes: 14 additions & 1 deletion classes/Rest/Controllers/BaseControllerProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,12 +284,25 @@ private function getParam(Request $request, $name, $mandatory, $default, $filter
}
}

// If the parameter is an array, throw an exception.
$invalidMessage = (
"Invalid value for $name. Must be a(n) $expectedValueType."
);
if (is_array($value)) {
throw new BadRequestHttpException($invalidMessage);
}

// Run the found parameter value through the given filter.
if (array_key_exists('flags', $filterOptions)) {
$filterOptions['flags'] |= FILTER_NULL_ON_FAILURE;
} else {
$filterOptions['flags'] = FILTER_NULL_ON_FAILURE;
}
$value = filter_var($value, $filterId, $filterOptions);

// If the value is invalid, throw an exception.
if ($value === null) {
throw new BadRequestHttpException("Invalid value for $name. Must be a(n) $expectedValueType.");
throw new BadRequestHttpException($invalidMessage);
}

// Return the filtered value.
Expand Down
6 changes: 5 additions & 1 deletion classes/Rest/XdmodApplicationFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,11 @@ public static function getInstance()
// Extracting any POST variables provided in the Request.
$post = array();
foreach($request->request->getIterator() as $key => $value) {
$post[$key] = json_decode($value, true);
$post[$key] = (
is_string($value)
? json_decode($value, true)
: null
);
}

// Calculate the amount of time that has elapsed serving this request.
Expand Down
127 changes: 97 additions & 30 deletions tests/integration/lib/BaseTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@
* authentication failures give the correct response, failing to authorize as a
* center director gives the correct response, requests in which the 'limit' or
* 'start_date' parameters are missing give the correct response, and requests
* in which the value of 'limit' is not a valid integer or 'start_date' is not
* a valid ISO 8601 date give the correct response.
* give the correct response for which the value of 'limit' is not a valid
* integer, the values of 'realm' or 'dimension' are not valid strings, the
* value of 'ts' is not a valid Unix timestamp, or the value of 'start_date' is
* not a valid ISO 8601 date.
*
* public function testGetData($id, $role, $input, $output)
* {
Expand Down Expand Up @@ -62,7 +64,9 @@
* 'authentication' => true,
* 'authorization' => 'cd',
* 'int_params' => ['limit'],
* 'date_params' => ['start_date']
* 'string_params' => ['realm', 'dimension'],
* 'unix_ts_params' => ['ts'],
* 'date_params' => ['start_date'],
* ]
* );
* }
Expand Down Expand Up @@ -394,6 +398,10 @@ protected static function assertRequiredKeys(
* need to be present for other tests here to succeed.
* - 'int_params' — array of parameters that will each be tested for
* invalid integer values.
* - 'string_params' — array of parameters that will each be tested for
* invalid string values.
* - 'unix_ts_params' — array of parameters that will each be tested for
* invalid Unix timestamp values.
* - 'date_params' — array of parameters that will each be tested for
* invalid ISO 8601 date values.
* @return array of arrays of test data, each of which contains a string ID of the test, a
Expand Down Expand Up @@ -423,6 +431,11 @@ protected function provideRestEndpointTests(
$options['additional_params']
);
}
// Set up the custom error body validator.
$errorBodyValidator = null;
if (array_key_exists('error_body_validator', $options)) {
$errorBodyValidator = $options['error_body_validator'];
}
$tests = [];
// Provide authentication tests.
if (
Expand All @@ -433,7 +446,10 @@ protected function provideRestEndpointTests(
'unauthenticated',
'pub',
$validInputWithAdditionalParams,
$this->validateAuthorizationErrorResponse(401)
$this->validateAuthorizationErrorResponse(
401,
$errorBodyValidator
)
];
}
// Provide authorization tests.
Expand All @@ -444,7 +460,10 @@ protected function provideRestEndpointTests(
'unauthorized',
$role,
$validInputWithAdditionalParams,
$this->validateAuthorizationErrorResponse(403)
$this->validateAuthorizationErrorResponse(
403,
$errorBodyValidator
)
];
}
}
Expand Down Expand Up @@ -488,30 +507,48 @@ protected function provideRestEndpointTests(
array_push(
$testData,
$input,
$this->validateMissingRequiredParameterResponse($param)
$this->validateMissingRequiredParameterResponse(
$param,
$errorBodyValidator
)
);
$tests[] = $testData;
}
// Provide tests of invalid parameters.
$types = [
'int_params' => 'integer',
'string_params' => 'string',
'unix_ts_params' => 'Unix timestamp',
'date_params' => 'ISO 8601 Date'
];
$values = [
'string' => 'foo',
'array' => ['foo' => 'bar']
];
foreach ($types as $key => $type) {
if (array_key_exists($key, $options)) {
foreach ($options[$key] as $param) {
$input = $validInputWithAdditionalParams;
$input[$paramSource][$param] = 'foo';
$testData = ["{$param}_string", $runAs];
if ($tokenAuth) {
$testData[] = 'valid_token';
foreach ($values as $id => $value) {
// Strings can be strings, so skip that test.
if ('string_params' !== $key || 'string' !== $id) {
$input[$paramSource][$param] = $value;
$testData = ["{$param}_$id", $runAs];
if ($tokenAuth) {
$testData[] = 'valid_token';
}
array_push(
$testData,
$input,
$this->validateInvalidParameterResponse(
$param,
$type,
$errorBodyValidator
)
);
$tests[] = $testData;
}
}
array_push(
$testData,
$input,
$this->validateInvalidParameterResponse($param, $type)
);
$tests[] = $testData;
}
}
}
Expand All @@ -524,13 +561,18 @@ protected function provideRestEndpointTests(
* the given name was not provided in the request.
*
* @param string $name
* @param callable|null $bodyValidator if provided, overrides the default
* body validator.
* @return array
*/
protected function validateMissingRequiredParameterResponse($name)
{
protected function validateMissingRequiredParameterResponse(
$name,
$bodyValidator = null
) {
return $this->validateBadRequestResponse(
"$name is a required parameter.",
0
0,
$bodyValidator
);
}

Expand All @@ -541,13 +583,19 @@ protected function validateMissingRequiredParameterResponse($name)
*
* @param string $name
* @param string $type
* @param callable|null $bodyValidator if provided, overrides the default
* body validator.
* @return array
*/
protected function validateInvalidParameterResponse($name, $type)
{
protected function validateInvalidParameterResponse(
$name,
$type,
$bodyValidator = null
) {
return $this->validateBadRequestResponse(
"Invalid value for $name. Must be a(n) $type.",
0
0,
$bodyValidator
);
}

Expand All @@ -558,15 +606,21 @@ protected function validateInvalidParameterResponse($name, $type)
*
* @param string $message
* @param int $code
* @param callable|null $bodyValidator if provided, overrides the default
* body validator.
* @return array
*/
protected function validateBadRequestResponse($message, $code)
{
protected function validateBadRequestResponse(
$message,
$code,
$bodyValidator = null
) {
return [
'status_code' => 400,
'body_validator' => $this->validateErrorResponseBody(
$message,
$code
$code,
$bodyValidator
)
];
}
Expand All @@ -576,18 +630,23 @@ protected function validateBadRequestResponse($message, $code)
* validates authorization error responses with the given HTTP status code.
*
* @param int $statusCode
* @param callable|null $bodyValidator if provided, overrides the default
* body validator.
* @return array
*/
protected function validateAuthorizationErrorResponse($statusCode)
{
protected function validateAuthorizationErrorResponse(
$statusCode,
$bodyValidator = null
) {
return [
'status_code' => $statusCode,
'body_validator' => $this->validateErrorResponseBody(
(
'An error was encountered while attempting to process the'
. ' requested authorization procedure.'
),
0
0,
$bodyValidator
)
];
}
Expand All @@ -599,10 +658,18 @@ protected function validateAuthorizationErrorResponse($statusCode)
*
* @param string $message
* @param int $code
* @param callable|null $bodyValidator if provided, overrides the default
* body validator.
* @return callable
*/
protected function validateErrorResponseBody($message, $code)
{
protected function validateErrorResponseBody(
$message,
$code,
$bodyValidator = null
) {
if (!is_null($bodyValidator)) {
return $bodyValidator($message, $code);
}
return function ($body, $assertMessage) use ($message, $code) {
parent::assertEquals(
[
Expand Down
Loading

0 comments on commit d4dedd6

Please sign in to comment.