Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix validation of REST array parameters. #1768

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Fix validation of REST array parameters. #1768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4dedd6
Fix validation of REST array parameters.
aaronweeden Dec 7, 2023
fdc382b
Add ability to do `PUT` requests with `XdmodTestHelper`.
aaronweeden Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion classes/Rest/Controllers/BaseControllerProvider.php
View file
Open in desktop
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
Open in desktop
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
135 changes: 102 additions & 33 deletions tests/integration/lib/BaseTest.php
View file
Open in desktop
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 @@ -142,7 +146,7 @@ function ($role) {
* - 'path': the URL path to the HTTP endpoint (e.g.,
* '/rest/...').
* - 'method': the HTTP method, either 'get', 'post',
* 'delete', or 'patch'.
* 'put', 'delete', or 'patch'.
* - 'params': associative array of query parameters.
* - 'data': associative array of request body data.
* @return array element 0 is the response body, element 1 is the return of
Expand All @@ -165,6 +169,7 @@ public static function makeHttpRequest(
);
break;
case 'post':
case 'put':
case 'delete':
case 'patch':
$response = $testHelper->$method(
Expand Down Expand Up @@ -359,8 +364,8 @@ protected static function assertRequiredKeys(
* 'params' or 'data' key is mapped to an associative array in which
* the keys are all of the required endpoint parameters, and the values
* are valid values for those parameters. If the 'method' value is
* 'post' or 'patch', the parameters will be pulled from the 'data'
* value; otherwise, they will be pulled from the 'params' value.
* 'post', 'put', or 'patch', the parameters will be pulled from the
* 'data' value; otherwise, they will be pulled from the 'params' value.
* @param array $options an associative array that configures how this method should run.
* The keys are all optional:
* - 'authentication' — if the value is true, the return will include a
Expand Down Expand Up @@ -394,6 +399,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 All @@ -410,6 +419,7 @@ protected function provideRestEndpointTests(
$paramSource = 'params';
if (
'post' === $validInput['method']
|| 'put' === $validInput['method']
|| 'patch' === $validInput['method']
) {
$paramSource = 'data';
Expand All @@ -423,6 +433,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 +448,10 @@ protected function provideRestEndpointTests(
'unauthenticated',
'pub',
$validInputWithAdditionalParams,
$this->validateAuthorizationErrorResponse(401)
$this->validateAuthorizationErrorResponse(
401,
$errorBodyValidator
)
];
}
// Provide authorization tests.
Expand All @@ -444,7 +462,10 @@ protected function provideRestEndpointTests(
'unauthorized',
$role,
$validInputWithAdditionalParams,
$this->validateAuthorizationErrorResponse(403)
$this->validateAuthorizationErrorResponse(
403,
$errorBodyValidator
)
];
}
}
Expand Down Expand Up @@ -488,30 +509,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 +563,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 +585,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 +608,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 +632,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 +660,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