Profile EULA view: Fixed #19344 - fixed permissions on downloading EULAS

app/Http/Controllers/ProfileController.php

@@ -265,14 +265,24 @@ class ProfileController extends Controller
     {
         $filename = basename((string) $filename);
 
-        $logentry = Actionlog::where('filename', $filename)->with('user', 'target')->first();
+        $logentry = Actionlog::where('filename', $filename)
+            ->where('action_type', 'accepted')
+            ->with('target')
+            ->first();
 
         if (! $logentry) {
             return redirect()->back()->with('error', trans('general.record_not_found'));
         }
 
-        $this->authorize('view', $logentry->target);
-        $this->authorize('view', $logentry->user);
+        // #19344: end users need to be able to download their own accepted
+        // EULA without holding users.view. Anyone else falls back to the
+        // regular view-user permission check.
+        $isOwnAcceptance = $logentry->target_type === User::class
+            && (int) $logentry->target_id === (int) auth()->id();
+
+        if (! $isOwnAcceptance) {
+            $this->authorize('view', $logentry->target);
+        }
 
         if (config('filesystems.default') == 's3_private') {
             return redirect()->away(Storage::disk('s3_private')->temporaryUrl('private_uploads/eula-pdfs/'.$filename, now()->addMinutes(5)));

tests/Feature/Account/GetStoredEulaTest.php

@@ -0,0 +1,128 @@
+<?php
+
+namespace Tests\Feature\Account;
+
+use App\Models\Actionlog;
+use App\Models\Asset;
+use App\Models\User;
+use Tests\TestCase;
+
+class GetStoredEulaTest extends TestCase
+{
+    public function test_requires_authentication(): void
+    {
+        // Give the setup middleware a user so it doesn't short-circuit
+        // us to /setup before auth has a chance to run.
+        User::factory()->create();
+
+        $this->get(route('profile.storedeula.download', ['filename' => 'anything.pdf']))
+            ->assertRedirect(route('login'));
+    }
+
+    public function test_nonexistent_filename_redirects_with_error(): void
+    {
+        $this->actingAs(User::factory()->create())
+            ->get(route('profile.storedeula.download', ['filename' => 'does-not-exist.pdf']))
+            ->assertRedirect(route('home'));
+    }
+
+    /**
+     * Regression for #19344: a regular end user without users.view
+     * still needs to be able to download the EULA PDF for an asset
+     * they themselves accepted. We can't verify the actual file bytes
+     * come back without wiring a real file into storage_path(), so we
+     * assert on the distinct downstream signal: authorization succeeds
+     * (no 403), storage lookup misses, controller redirects back with
+     * the "file does not exist" error. That's the exact response an
+     * end user would see if their EULA PDF had been purged.
+     */
+    public function test_end_user_can_reach_download_flow_for_their_own_accepted_eula(): void
+    {
+        $endUser = User::factory()->create();
+        $asset = Asset::factory()->create();
+        $filename = 'accepted-user-'.uniqid().'.pdf';
+
+        Actionlog::factory()->create([
+            'action_type' => 'accepted',
+            'item_id' => $asset->id,
+            'item_type' => Asset::class,
+            'target_id' => $endUser->id,
+            'target_type' => User::class,
+            'filename' => $filename,
+        ]);
+
+        $this->assertFalse($endUser->hasAccess('users.view'));
+
+        $this->from(route('account'))
+            ->actingAs($endUser)
+            ->get(route('profile.storedeula.download', ['filename' => $filename]))
+            ->assertRedirect(route('account'))
+            ->assertSessionHas('error', trans('general.file_does_not_exist'));
+    }
+
+    public function test_end_user_cannot_download_someone_elses_eula(): void
+    {
+        $acceptingUser = User::factory()->create();
+        $otherEndUser = User::factory()->create();
+        $asset = Asset::factory()->create();
+        $filename = 'someone-elses-'.uniqid().'.pdf';
+
+        Actionlog::factory()->create([
+            'action_type' => 'accepted',
+            'item_id' => $asset->id,
+            'item_type' => Asset::class,
+            'target_id' => $acceptingUser->id,
+            'target_type' => User::class,
+            'filename' => $filename,
+        ]);
+
+        $this->actingAs($otherEndUser)
+            ->get(route('profile.storedeula.download', ['filename' => $filename]))
+            ->assertForbidden();
+    }
+
+    public function test_user_with_users_view_permission_can_reach_download_flow_for_any_eula(): void
+    {
+        $acceptingUser = User::factory()->create();
+        $manager = User::factory()->viewUsers()->create();
+        $asset = Asset::factory()->create();
+        $filename = 'manager-download-'.uniqid().'.pdf';
+
+        Actionlog::factory()->create([
+            'action_type' => 'accepted',
+            'item_id' => $asset->id,
+            'item_type' => Asset::class,
+            'target_id' => $acceptingUser->id,
+            'target_type' => User::class,
+            'filename' => $filename,
+        ]);
+
+        $this->from(route('account'))
+            ->actingAs($manager)
+            ->get(route('profile.storedeula.download', ['filename' => $filename]))
+            ->assertRedirect(route('account'))
+            ->assertSessionHas('error', trans('general.file_does_not_exist'));
+    }
+
+    public function test_filename_that_matches_a_non_acceptance_log_row_is_rejected(): void
+    {
+        $endUser = User::factory()->create();
+        $filename = 'not-a-eula-'.uniqid().'.pdf';
+
+        // A log row with the same filename but action_type != accepted
+        // (e.g. a general file upload) must not be treated as an EULA
+        // the end user is entitled to.
+        Actionlog::factory()->create([
+            'action_type' => 'uploaded',
+            'item_id' => $endUser->id,
+            'item_type' => User::class,
+            'target_id' => $endUser->id,
+            'target_type' => User::class,
+            'filename' => $filename,
+        ]);
+
+        $this->actingAs($endUser)
+            ->get(route('profile.storedeula.download', ['filename' => $filename]))
+            ->assertRedirect(route('home'));
+    }
+}