Ensure we can properly create an activity log entry; always return soft-deleted models

Author: DaneEveritt

app/Events/ActivityLogged.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Pterodactyl\Events;
+
+use Illuminate\Support\Str;
+use Pterodactyl\Models\ActivityLog;
+use Illuminate\Database\Eloquent\Model;
+
+class ActivityLogged extends Event
+{
+    public ActivityLog $model;
+
+    public function __construct(ActivityLog $model)
+    {
+        $this->model = $model;
+    }
+
+    public function is(string $event): bool
+    {
+        return $this->model->event === $event;
+    }
+
+    public function actor(): ?Model
+    {
+        return $this->isSystem() ? null : $this->model->actor;
+    }
+
+    public function isServerEvent()
+    {
+        return Str::startsWith($this->model->event, 'server:');
+    }
+
+    public function isSystem()
+    {
+        return is_null($this->model->actor_id);
+    }
+}

app/Models/ActivityLog.php

@@ -2,6 +2,8 @@
 
 namespace Pterodactyl\Models;
 
+use Illuminate\Support\Facades\Event;
+use Pterodactyl\Events\ActivityLogged;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Relations\MorphTo;
 use Illuminate\Database\Eloquent\Model as IlluminateModel;
@@ -16,13 +18,14 @@
  * @property string|null $description
  * @property string|null $actor_type
  * @property int|null $actor_id
- * @property \Illuminate\Support\Collection $properties
+ * @property \Illuminate\Support\Collection|null $properties
  * @property string $timestamp
  * @property IlluminateModel|\Eloquent $actor
- * @property IlluminateModel|\Eloquent $subject
+ * @property \Illuminate\Database\Eloquent\Collection|\Pterodactyl\Models\ActivityLogSubject[] $subjects
+ * @property int|null $subjects_count
  *
- * @method static Builder|ActivityLog forEvent(string $event)
  * @method static Builder|ActivityLog forActor(\Illuminate\Database\Eloquent\Model $actor)
+ * @method static Builder|ActivityLog forEvent(string $action)
  * @method static Builder|ActivityLog newModelQuery()
  * @method static Builder|ActivityLog newQuery()
  * @method static Builder|ActivityLog query()
@@ -50,6 +53,8 @@ class ActivityLog extends Model
         'properties' => 'collection',
     ];
 
+    protected $with = ['subjects'];
+
     public static $validationRules = [
         'event' => ['required', 'string'],
         'batch' => ['nullable', 'uuid'],
@@ -60,7 +65,12 @@ class ActivityLog extends Model
 
     public function actor(): MorphTo
     {
-        return $this->morphTo();
+        return $this->morphTo()->withTrashed();
+    }
+
+    public function subjects()
+    {
+        return $this->hasMany(ActivityLogSubject::class);
     }
 
     public function scopeForEvent(Builder $builder, string $action): Builder
@@ -75,4 +85,17 @@ public function scopeForActor(Builder $builder, IlluminateModel $actor): Builder
     {
         return $builder->whereMorphedTo('actor', $actor);
     }
+
+    /**
+     * Boots the model event listeners. This will trigger an activity log event every
+     * time a new model is inserted which can then be captured and worked with as needed.
+     */
+    protected static function boot()
+    {
+        parent::boot();
+
+        static::created(function (self $model) {
+            Event::dispatch(new ActivityLogged($model));
+        });
+    }
 }

app/Models/ActivityLogSubject.php

@@ -35,6 +35,6 @@ public function activityLog()
 
     public function subject()
     {
-        return $this->morphTo();
+        return $this->morphTo()->withTrashed();
     }
 }

app/Services/Activity/ActivityLogService.php

@@ -6,6 +6,7 @@
 use Webmozart\Assert\Assert;
 use Illuminate\Support\Collection;
 use Pterodactyl\Models\ActivityLog;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Contracts\Auth\Factory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Facades\Request;
@@ -132,7 +133,9 @@ public function withRequestMetadata(): self
 
     /**
      * Logs an activity log entry with the set values and then returns the
-     * model instance to the caller.
+     * model instance to the caller. If there is an exception encountered while
+     * performing this action it will be logged to the disk but will not interrupt
+     * the code flow.
      */
     public function log(string $description = null): ActivityLog
     {
@@ -142,7 +145,13 @@ public function log(string $description = null): ActivityLog
             $activity->description = $description;
         }
 
-        return $this->save();
+        try {
+            return $this->save();
+        } catch (\Throwable|\Exception $exception) {
+            Log::error($exception);
+        }
+
+        return $activity;
     }
 
     /**
@@ -166,9 +175,9 @@ public function clone(): self
     public function transaction(\Closure $callback)
     {
         return $this->connection->transaction(function () use ($callback) {
-            $response = $callback($activity = $this->getActivity());
+            $response = $callback($this);
 
-            $this->save($activity);
+            $this->save();
 
             return $response;
         });
@@ -209,14 +218,12 @@ protected function getActivity(): ActivityLog
      *
      * @throws \Throwable
      */
-    protected function save(ActivityLog $activity = null): ActivityLog
+    protected function save(): ActivityLog
     {
-        $activity = $activity ?? $this->activity;
+        Assert::notNull($this->activity);
 
-        Assert::notNull($activity);
-
-        $response = $this->connection->transaction(function () use ($activity) {
-            $activity->save();
+        $response = $this->connection->transaction(function () {
+            $this->activity->save();
 
             $subjects = Collection::make($this->subjects)
                 ->map(fn (Model $subject) => [
@@ -229,7 +236,7 @@ protected function save(ActivityLog $activity = null): ActivityLog
 
             ActivityLogSubject::insert($subjects);
 
-            return $activity;
+            return $this->activity;
         });
 
         $this->activity = null;

tests/Integration/Api/Client/Server/Backup/DeleteBackupTest.php

@@ -5,8 +5,9 @@
 use Mockery;
 use Illuminate\Http\Response;
 use Pterodactyl\Models\Backup;
-use Pterodactyl\Models\AuditLog;
 use Pterodactyl\Models\Permission;
+use Illuminate\Support\Facades\Event;
+use Pterodactyl\Events\ActivityLogged;
 use Pterodactyl\Repositories\Wings\DaemonBackupRepository;
 use Pterodactyl\Tests\Integration\Api\Client\ClientApiIntegrationTestCase;
 
@@ -34,32 +35,41 @@ public function testUserWithoutPermissionCannotDeleteBackup()
     /**
      * Tests that a backup can be deleted for a server and that it is properly updated
      * in the database. Once deleted there should also be a corresponding record in the
-     * audit logs table for this API call.
+     * activity logs table for this API call.
      */
     public function testBackupCanBeDeleted()
     {
+        Event::fake([ActivityLogged::class]);
+
         [$user, $server] = $this->generateTestAccount([Permission::ACTION_BACKUP_DELETE]);
 
         /** @var \Pterodactyl\Models\Backup $backup */
         $backup = Backup::factory()->create(['server_id' => $server->id]);
 
-        $this->repository->expects('setServer->delete')->with(Mockery::on(function ($value) use ($backup) {
-            return $value instanceof Backup && $value->uuid === $backup->uuid;
-        }))->andReturn(new Response());
+        $this->repository->expects('setServer->delete')->with(
+            Mockery::on(function ($value) use ($backup) {
+                return $value instanceof Backup && $value->uuid === $backup->uuid;
+            })
+        )->andReturn(new Response());
 
         $this->actingAs($user)->deleteJson($this->link($backup))->assertStatus(Response::HTTP_NO_CONTENT);
 
         $backup->refresh();
+        $this->assertSoftDeleted($backup);
 
-        $this->assertNotNull($backup->deleted_at);
+        Event::assertDispatched(ActivityLogged::class, function (ActivityLogged $event) use ($backup, $user) {
+            $this->assertTrue($event->isServerEvent());
+            $this->assertTrue($event->is('server:backup.delete'));
+            $this->assertTrue($user->is($event->actor()));
+            $this->assertCount(2, $event->model->subjects);
 
-        $this->actingAs($user)->deleteJson($this->link($backup))->assertStatus(Response::HTTP_NOT_FOUND);
+            $subjects = $event->model->subjects;
+            $this->assertCount(1, $subjects->filter(fn ($model) => $model->subject->is($backup)));
+            $this->assertCount(1, $subjects->filter(fn ($model) => $model->subject->is($backup->server)));
 
-        $event = $backup->audits()->where('action', AuditLog::SERVER__BACKUP_DELETED)->latest()->first();
+            return true;
+        });
 
-        $this->assertNotNull($event);
-        $this->assertFalse($event->is_system);
-        $this->assertEquals($backup->server_id, $event->server_id);
-        $this->assertEquals($user->id, $event->user_id);
+        $this->actingAs($user)->deleteJson($this->link($backup))->assertStatus(Response::HTTP_NOT_FOUND);
     }
 }