Fix: update avatar route to check author of attachment (#7476)

Co-authored-by: Jon Waldstein <[email protected]>
impress-org · Aug 7, 2024 · a38b14c · a38b14c
1 parent bbf81b5
commit a38b14c
Show file tree

Hide file tree

Showing 4 changed files with 158 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,104 @@
+<?php
+
+namespace Give\Tests\Unit\DonorDashboards\Repositories;
+
+use Give\DonorDashboards\Profile;
+use Give\Donors\Models\Donor;
+use Give\Tests\TestCase;
+use Give\Tests\TestTraits\RefreshDatabase;
+
+/**
+ * @unreleased
+ */
+final class TestProfile extends TestCase
+{
+    use RefreshDatabase;
+
+    /**
+     * @unreleased
+     */
+    public function testAvatarBelongsToCurrentUserShouldReturnTrue(): 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());
+    }
+
+    /**
+     * @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());
+    }
+}