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

Enhance param validation on refreshToken #28

Merged
merged 4 commits into from
Feb 7, 2022
Merged

Enhance param validation on refreshToken #28

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b740d1c
enhance param validation on refreshToken
knazart Jan 28, 2022
1aec478
update the data cleaner
knazart Jan 28, 2022
8d1619b
remove debug code
knazart Jan 28, 2022
397df57
reuse the value
knazart Jan 28, 2022
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
38 changes: 20 additions & 18 deletions modules/EED_SquareOnsiteOAuth.module.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,20 +492,20 @@ public static function registerDomain(EE_Payment_Method $square_pm): array
* Refresh the access token.
*
* @param EE_Payment_Method $squarePm
* @return void
* @return bool
* @throws InvalidArgumentException
* @throws InvalidInterfaceException
* @throws InvalidDataTypeException
* @throws EE_Error
* @throws ReflectionException
* @throws Exception
*/
public static function refreshToken(EE_Payment_Method $squarePm)
public static function refreshToken(EE_Payment_Method $squarePm): bool
{
$squareData = $squarePm->get_extra_meta(Domain::META_KEY_SQUARE_DATA, true);
if (! isset($squareData[ Domain::META_KEY_REFRESH_TOKEN ]) || ! $squareData[ Domain::META_KEY_REFRESH_TOKEN ]) {
$errMsg = esc_html__('Could not find the refresh token.', 'event_espresso');
EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $errMsg, $squareData, false);
return EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $errMsg, $squareData, false);
}
$squareRefreshToken = EED_SquareOnsiteOAuth::decryptString(
$squareData[ Domain::META_KEY_REFRESH_TOKEN ],
Expand Down Expand Up @@ -535,11 +535,12 @@ public static function refreshToken(EE_Payment_Method $squarePm)
$response = wp_remote_post($postUrl, $postArgs);

if (is_wp_error($response)) {
EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $response->get_error_message(), [], false);
return EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $response->get_error_message(), [], false);
} else {
$responseBody = (isset($response['body']) && $response['body']) ? json_decode($response['body']) : false;
// $responseBody = (isset($response['body']) && $response['body']) ? json_decode($response['body']) : false;
$responseBody = null;
if (
$responseBody === false
! $responseBody
|| (
isset($responseBody->error)
&& strpos($responseBody->error_description, 'This application is not connected') === false
Expand All @@ -550,11 +551,12 @@ public static function refreshToken(EE_Payment_Method $squarePm)
} else {
$errMsg = esc_html__('Unknown response received!', 'event_espresso');
}
EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $errMsg, (array) $responseBody, false);
return EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $errMsg, (array) $responseBody, false);
}

if (
! wp_verify_nonce($responseBody->nonce, 'eea_square_refresh_access_token')
empty($responseBody->nonce)
|| ! wp_verify_nonce($responseBody->nonce, 'eea_square_refresh_access_token')
|| empty($responseBody->expires_at)
|| empty($responseBody->application_id)
|| empty($responseBody->access_token)
Expand All @@ -563,7 +565,7 @@ public static function refreshToken(EE_Payment_Method $squarePm)
) {
// This is an error.
$errMsg = esc_html__('Could not get the refresh token and/or other parameters.', 'event_espresso');
EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $errMsg, (array) $responseBody, false);
return EED_SquareOnsiteOAuth::errorLogAndExit($squarePm, $errMsg, (array) $responseBody, false);
}

// log the response, don't exit
Expand Down Expand Up @@ -606,7 +608,7 @@ public static function refreshToken(EE_Payment_Method $squarePm)
$locationsList = EED_SquareOnsiteOAuth::updateLocationsList($squarePm);
// Did we really get an error ?
if (isset($locationsList['error'])) {
EED_SquareOnsiteOAuth::errorLogAndExit(
return EED_SquareOnsiteOAuth::errorLogAndExit(
$squarePm,
$locationsList['error']['message'],
(array) $locationsList,
Expand All @@ -617,6 +619,7 @@ public static function refreshToken(EE_Payment_Method $squarePm)
// And update the location ID if not set.
EED_SquareOnsiteOAuth::updateLocation($squarePm, $locationsList);
}
return true;
}


Expand Down Expand Up @@ -837,15 +840,12 @@ public static function decryptString(string $text, bool $sandbox_mode): string
*/
private static function cleanDataArray(array $data): array
{
$sensitive_data = [
'access_token' => '',
'refresh_token' => '',
'nonce' => '',
];
$sensitive_data = ['access_token', 'refresh_token', 'nonce'];
foreach ($data as $key => $value) {
$data[ $key ] = is_array($value) ? EED_SquareOnsiteOAuth::cleanDataArray($value) : $value;
$data[ $key ] = in_array($key, $sensitive_data) ? '***' : $value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this second line is overwriting the previous one and therefore negating what it has done

what about something like:

Suggested change
$data[ $key ] = is_array($value) ? EED_SquareOnsiteOAuth::cleanDataArray($value) : $value;
$data[ $key ] = in_array($key, $sensitive_data) ? '***' : $value;
$value = is_array($value) ? EED_SquareOnsiteOAuth::cleanDataArray($value) : $value;
$data[ $key ] = in_array($key, $sensitive_data) ? '***' : $value;

}
return array_diff_key($data, $sensitive_data);
return $data;
}


Expand All @@ -858,7 +858,7 @@ private static function cleanDataArray(array $data): array
* @param bool $echo_json_and_exit Should we echo json and exit
* @param bool $using_oauth
* @param bool $show_alert Tells frontend to show an alert or not
* @return void
* @return bool
* @throws EE_Error
*/
public static function errorLogAndExit(
Expand All @@ -868,7 +868,7 @@ public static function errorLogAndExit(
bool $echo_json_and_exit = true,
bool $using_oauth = false,
bool $show_alert = false
) {
): bool {
$default_msg = 'Square error';
if ($square_pm instanceof EE_Payment_Method) {
if ($data) {
Expand All @@ -889,6 +889,8 @@ public static function errorLogAndExit(
self::echoJsonError($err_msg, $show_alert);
}
}
// Yes, simply always return true.
return true;
}


Expand Down