Fix: Ensure Event/Volunteer Responses only Include Intended User Fields

app/Http/Controllers/API/UserController.php

@@ -57,15 +57,9 @@ protected static function getUserAudits($dateFrom = null)
 
     protected static function mapUserAndAuditToUserChange($user, $audit)
     {
-        // Hide fields not relevant for Zapier.
         $user->makeHidden([
             'updated_at',
             'deleted_at',
-            'api_token',
-            'recovery',
-            'recovery_expires',
-            'calendar_hash',
-            'number_of_logins',
             'consent_past_data',
             'consent_gdpr',
             'consent_future_data',

app/Http/Controllers/ApiController.php

@@ -164,8 +164,6 @@ public static function getUserInfo(): JsonResponse
     {
         $user = Auth::user();
 
-        $user->makeHidden('api_token');
-
         return response()->json($user->toArray());
     }
 

app/Models/Party.php

@@ -818,27 +818,34 @@ public static function expandVolunteers($volunteers, $showEmails) {
         $ret = [];
 
         foreach ($volunteers as $volunteer) {
-            $volunteer['userSkills'] = [];
             $volunteer['confirmed'] = intval($volunteer->status) === 1;
             $volunteer['profilePath'] = '/uploads/thumbnail_placeholder.png';
             $volunteer['fullName'] = $volunteer->getFullName();
 
             if ($volunteer->volunteer) {
-                $volunteer['volunteer'] = $volunteer->volunteer;
+                $user = $volunteer->volunteer;
 
-                if (!$showEmails) {
-                    $volunteer['volunteer']['email'] = NULL;
-                }
-
-                if (! empty($volunteer->volunteer)) {
-                    $volunteer['userSkills'] = $volunteer->volunteer->userSkills->all();
-                    $volunteer['profilePath'] = '/uploads/thumbnail_'.$volunteer->volunteer->getProfile($volunteer->volunteer->id)->path;
+                $userSkills = $user->userSkills->all();
+                $volunteer['profilePath'] = '/uploads/thumbnail_'.$user->getProfile($user->id)->path;
 
-                    foreach ($volunteer['userSkills'] as $skill) {
-                        // Force expansion
-                        $skill->skillName->skill_name;
-                    }
+                foreach ($userSkills as $skill) {
+                    $skill->skillName->skill_name;
                 }
+
+                // Drop the loaded Eloquent relation before overwriting it with the
+                // allowlist. Otherwise relationsToArray() re-injects the full User
+                // model during serialization, overriding the array set below and
+                // leaking the entire user record to public event pages.
+                $volunteer->unsetRelation('volunteer');
+                $volunteer['volunteer'] = [
+                    'id' => $user->id,
+                    'name' => $user->name,
+                    'email' => $showEmails ? $user->email : null,
+                    // The event page reads volunteer.user_skills (see
+                    // EventAttendee.vue); keep it in the allowlist so the skills
+                    // list still renders. Skill associations are not sensitive.
+                    'user_skills' => $userSkills,
+                ];
             }
 
             $ret[] = $volunteer;

app/Models/User.php

@@ -54,7 +54,16 @@ class User extends Authenticatable implements Auditable, HasLocalePreference
      * @var array
      */
     protected $hidden = [
-        'password', 'remember_token',
+        'password',
+        'remember_token',
+        'api_token',
+        'calendar_hash',
+        'recovery',
+        'recovery_expires',
+        'latitude',
+        'longitude',
+        'last_login_at',
+        'number_of_logins',
     ];
 
     /**

tests/Feature/Events/PublicEventDataLeakTest.php

@@ -0,0 +1,171 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Models\EventsUsers;
+use App\Models\Group;
+use App\Models\Network;
+use App\Models\Party;
+use App\Models\Role;
+use App\Models\User;
+use Illuminate\Support\Facades\Queue;
+use Tests\TestCase;
+
+class PublicEventDataLeakTest extends TestCase
+{
+    // Field-name sentinels for keys that are user-only and must never be
+    // serialized. latitude/longitude are deliberately NOT listed here: they
+    // are legitimate public fields on the event itself (Party model), so the
+    // bare key appears in responses for the event's location. The volunteer's
+    // own coordinates are guarded by the numeric value sentinels below instead.
+    private array $sensitiveFields = [
+        'api_token',
+        'calendar_hash',
+        'recovery',
+        'recovery_expires',
+    ];
+
+    private function createEventWithVolunteer(array $userOverrides = []): array
+    {
+        Queue::fake();
+
+        // The group must be approved, otherwise events on it are not visible to
+        // anonymous visitors (PartyController::view aborts with 404) and the
+        // public-page leak assertions below would never execute.
+        $group = Group::factory()->create(['approved' => true]);
+        $network = Network::factory()->create();
+        $network->addGroup($group);
+
+        $event = Party::factory()->create([
+            'group' => $group,
+            'event_start_utc' => '2130-01-01T12:13:00+00:00',
+            'event_end_utc' => '2130-01-01T13:14:00+00:00',
+        ]);
+
+        $volunteer = User::factory()->create(array_merge([
+            'api_token' => 'SENSITIVE_TOKEN_VALUE',
+            'calendar_hash' => 'SENSITIVE_CAL_HASH',
+            'latitude' => 51.50741234,
+            'longitude' => -0.12781234,
+            'recovery' => 'SENSITIVE_RECOVERY_TOKEN',
+            'recovery_expires' => now()->addHour(),
+            'username' => 'SENSITIVE_USERNAME',
+            'external_user_id' => 'SENSITIVE_EXT_ID',
+        ], $userOverrides));
+
+        EventsUsers::create([
+            'event' => $event->idevents,
+            'user' => $volunteer->id,
+            'status' => 1,
+            'role' => Role::RESTARTER,
+        ]);
+
+        return [$event, $volunteer];
+    }
+
+    private function assertNoSensitiveFields(string $content): void
+    {
+        // Props are json_encode()d then HTML-escaped by Blade ({{ }}) on the
+        // public page path, so the raw content contains &quot;api_token&quot;
+        // rather than "api_token". Decode entities first, otherwise these
+        // field-name checks are no-ops there.
+        $decoded = html_entity_decode($content, ENT_QUOTES);
+
+        foreach ($this->sensitiveFields as $field) {
+            $this->assertStringNotContainsString(
+                '"' . $field . '"',
+                $decoded,
+                "Sensitive field '$field' found in response"
+            );
+        }
+
+        $this->assertStringNotContainsString('SENSITIVE_TOKEN_VALUE', $content);
+        $this->assertStringNotContainsString('SENSITIVE_CAL_HASH', $content);
+        $this->assertStringNotContainsString('SENSITIVE_RECOVERY_TOKEN', $content);
+
+        // latitude/longitude have no string token to match on, so assert on
+        // their distinctive numeric values. These guard against the coordinates
+        // leaking even if the allowlist assertion is ever weakened.
+        $this->assertStringNotContainsString('51.50741234', $content, 'Volunteer latitude leaked');
+        $this->assertStringNotContainsString('0.12781234', $content, 'Volunteer longitude leaked');
+    }
+
+    /**
+     * Assert that every embedded "volunteer" object is limited to the
+     * {id, name, email} allowlist. The full User model used to leak here
+     * because a loaded Eloquent relation overrides the allowlist array
+     * during serialization, so checking only the $hidden fields above is
+     * not enough — username, external ids, location, age, consent dates,
+     * etc. would still be exposed.
+     */
+    private function assertVolunteerObjectsAllowlistOnly(string $content): void
+    {
+        // Props are json_encode()d then HTML-escaped by Blade ({{ }}), so
+        // decode entities before extracting the embedded JSON.
+        $decoded = html_entity_decode($content, ENT_QUOTES);
+
+        $this->assertMatchesRegularExpression(
+            '/"volunteer":\{/',
+            $decoded,
+            'No volunteer object was rendered; the allowlist assertions would be vacuous'
+        );
+
+        preg_match_all('/"volunteer":(\{[^}]*\})/', $decoded, $matches);
+
+        foreach ($matches[1] as $json) {
+            $object = json_decode($json, true);
+            $this->assertIsArray($object, "Could not decode volunteer object: $json");
+
+            $keys = array_keys($object);
+            sort($keys);
+
+            // The test volunteer has no skills, so user_skills is an empty array
+            // and the volunteer object stays flat (the regex above assumes that).
+            $this->assertSame(
+                ['email', 'id', 'name', 'user_skills'],
+                $keys,
+                'Volunteer object must be limited to the {id, name, email, user_skills} allowlist: ' . $json
+            );
+        }
+
+        // Sentinel PII values that must never reach any serialized response.
+        $this->assertStringNotContainsString('SENSITIVE_USERNAME', $content);
+        $this->assertStringNotContainsString('SENSITIVE_EXT_ID', $content);
+    }
+
+    public function testPublicEventPageDoesNotLeakSensitiveUserFields(): void
+    {
+        [$event, $volunteer] = $this->createEventWithVolunteer();
+
+        $response = $this->get('/party/view/' . $event->idevents);
+
+        $response->assertStatus(200);
+        $content = $response->getContent();
+
+        $this->assertNoSensitiveFields($content);
+        $this->assertVolunteerObjectsAllowlistOnly($content);
+
+        // Anonymous viewers must not see volunteer email addresses.
+        $this->assertStringNotContainsString(
+            $volunteer->email,
+            html_entity_decode($content, ENT_QUOTES),
+            'Volunteer email address exposed to an anonymous viewer'
+        );
+    }
+
+    public function testVolunteersApiDoesNotLeakSensitiveUserFields(): void
+    {
+        [$event] = $this->createEventWithVolunteer();
+
+        $admin = $this->createUserWithToken(Role::ADMINISTRATOR);
+        $this->actingAs($admin);
+
+        $response = $this->get('/api/events/' . $event->idevents . '/volunteers');
+
+        $response->assertStatus(200);
+        $content = $response->getContent();
+
+        $this->assertNoSensitiveFields($content);
+        $this->assertVolunteerObjectsAllowlistOnly($content);
+    }
+}