From a38b14c1c769ecfb064c34025d96d407052d9a70 Mon Sep 17 00:00:00 2001 From: Jon Waldstein Date: Wed, 7 Aug 2024 08:41:48 -0400 Subject: [PATCH] Fix: update avatar route to check author of attachment (#7476) Co-authored-by: Jon Waldstein --- .../Pipeline/Stages/UpdateDonorAvatar.php | 38 +++++-- src/DonorDashboards/Profile.php | 8 ++ .../Tabs/EditProfileTab/AvatarRoute.php | 19 +++- .../Repositories/TestProfile.php | 104 ++++++++++++++++++ 4 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 tests/Unit/DonorDashboards/Repositories/TestProfile.php diff --git a/src/DonorDashboards/Pipeline/Stages/UpdateDonorAvatar.php b/src/DonorDashboards/Pipeline/Stages/UpdateDonorAvatar.php index 328b948b35..3e1ce4cc08 100644 --- a/src/DonorDashboards/Pipeline/Stages/UpdateDonorAvatar.php +++ b/src/DonorDashboards/Pipeline/Stages/UpdateDonorAvatar.php @@ -2,7 +2,11 @@ namespace Give\DonorDashboards\Pipeline\Stages; +use Give\Log\Log; +use WP_REST_Response; + /** + * @unreleased added security measures to updateAvatarInMetaDB * @since 2.10.0 */ class UpdateDonorAvatar implements Stage @@ -23,14 +27,32 @@ public function __invoke($payload) protected function updateAvatarInMetaDB() { - $attributeMetaMap = [ - 'avatarId' => '_give_donor_avatar_id', - ]; - - foreach ($attributeMetaMap as $attribute => $metaKey) { - if (key_exists($attribute, $this->data)) { - $this->donor->update_meta($metaKey, $this->data[$attribute]); - } + if (!array_key_exists('avatarId', $this->data)) { + return false; + } + + $avatarId = $this->data['avatarId']; + + if (give()->donorDashboard->getAvatarId() && !give()->donorDashboard->avatarBelongsToCurrentUser($avatarId)) { + Log::error( + 'Avatar update permission denied.', + [ + 'donorId' => give()->donorDashboard->getId(), + 'avatarId' => give()->donorDashboard->getAvatarId() + ] + ); + + return new WP_REST_Response( + [ + 'status' => 401, + 'response' => 'unauthorized', + 'body_response' => [ + 'message' => __('Avatar update permission denied.', 'give'), + ], + ] + ); } + + return $this->donor->update_meta('_give_donor_avatar_id', $avatarId); } } diff --git a/src/DonorDashboards/Profile.php b/src/DonorDashboards/Profile.php index bad476ec46..90f3e15e9e 100644 --- a/src/DonorDashboards/Profile.php +++ b/src/DonorDashboards/Profile.php @@ -188,4 +188,12 @@ public function getCountry() return give_get_country(); } } + + /** + * @unreleased + */ + public function avatarBelongsToCurrentUser(?int $avatarId = null): bool + { + return (int)get_post_field("post_author", $avatarId ?? $this->getAvatarId()) === get_current_user_id(); + } } diff --git a/src/DonorDashboards/Tabs/EditProfileTab/AvatarRoute.php b/src/DonorDashboards/Tabs/EditProfileTab/AvatarRoute.php index bdf4e7b7cb..581896652a 100644 --- a/src/DonorDashboards/Tabs/EditProfileTab/AvatarRoute.php +++ b/src/DonorDashboards/Tabs/EditProfileTab/AvatarRoute.php @@ -7,6 +7,7 @@ use WP_REST_Response; /** + * @unreleased added security measure avatarBelongsToCurrentUser to handleRequest * @since 2.10.3 */ class AvatarRoute extends RouteAbstract @@ -35,7 +36,7 @@ public function args() */ public function handleRequest(WP_REST_Request $request) { - if ( ! (is_array($_POST) && is_array($_FILES))) { + if (!(is_array($_POST) && is_array($_FILES))) { return new WP_REST_Response( [ 'status' => 400, @@ -49,10 +50,23 @@ public function handleRequest(WP_REST_Request $request) // Delete existing Donor profile avatar attachment if (give()->donorDashboard->getAvatarId()) { + if (!give()->donorDashboard->avatarBelongsToCurrentUser()) { + return new WP_REST_Response( + [ + 'status' => 401, + 'response' => 'unauthorized', + 'body_response' => [ + 'message' => __('Permission denied.', 'give'), + ], + ] + ); + } + wp_delete_attachment(give()->donorDashboard->getAvatarId(), true); } - if ( ! function_exists('wp_handle_upload')) { + + if (!function_exists('wp_handle_upload')) { require_once(ABSPATH . 'wp-admin/includes/file.php'); } @@ -105,5 +119,4 @@ public function handleRequest(WP_REST_Request $request) ] ); } - } diff --git a/tests/Unit/DonorDashboards/Repositories/TestProfile.php b/tests/Unit/DonorDashboards/Repositories/TestProfile.php new file mode 100644 index 0000000000..06bd658d37 --- /dev/null +++ b/tests/Unit/DonorDashboards/Repositories/TestProfile.php @@ -0,0 +1,104 @@ +user->create_and_get(); + + /** @var Donor $donor */ + $donor = Donor::factory()->create([ + 'userId' => $user->ID, + ]); + + wp_set_current_user($donor->userId); + + $attachment = self::factory()->attachment->create_and_get([ + 'post_author' => $donor->userId, + 'post_title' => 'test', + 'post_content' => 'test', + 'post_status' => 'inherit', + 'post_mime_type' => 'image/jpeg', + ]); + + give()->donor_meta->update_meta($donor->id, '_give_donor_avatar_id', $attachment->ID); + + $profileRepository = new Profile(); + + $this->assertTrue($profileRepository->avatarBelongsToCurrentUser()); + } + + /** + * @unreleased + */ + public function testAvatarBelongsToCurrentUserShouldReturnTrueWithAvatarParam(): void + { + $user = self::factory()->user->create_and_get(); + + /** @var Donor $donor */ + $donor = Donor::factory()->create([ + 'userId' => $user->ID, + ]); + + wp_set_current_user($donor->userId); + + $attachment = self::factory()->attachment->create_and_get([ + 'post_author' => $donor->userId, + 'post_title' => 'test', + 'post_content' => 'test', + 'post_status' => 'inherit', + 'post_mime_type' => 'image/jpeg', + ]); + + give()->donor_meta->update_meta($donor->id, '_give_donor_avatar_id', $attachment->ID); + + $profileRepository = new Profile(); + + $this->assertTrue($profileRepository->avatarBelongsToCurrentUser($attachment->ID)); + } + + /** + * @unreleased + */ + public function testAvatarBelongsToCurrentUserShouldReturnFalse(): void + { + $user = self::factory()->user->create_and_get(); + + /** @var Donor $donor */ + $donor = Donor::factory()->create([ + 'userId' => $user->ID, + ]); + + wp_set_current_user($donor->userId); + + $attachment = self::factory()->attachment->create_and_get([ + 'post_author' => $donor->userId + 1, // Different user + 'post_title' => 'test', + 'post_content' => 'test', + 'post_status' => 'inherit', + 'post_mime_type' => 'image/jpeg', + ]); + + give()->donor_meta->update_meta($donor->id, '_give_donor_avatar_id', $attachment->ID); + + + $profileRepository = new Profile(); + + $this->assertFalse($profileRepository->avatarBelongsToCurrentUser()); + } +}