Remove legacy action format support and enforce migration (#148)

Author: cesargb

src/Actions/ResponseAction.php

@@ -7,7 +7,7 @@
 use Illuminate\Http\RedirectResponse;
 use Illuminate\View\View;
 use Laravel\SerializableClosure\SerializableClosure;
-use Laravel\SerializableClosure\Serializers\Signed;
+use MagicLink\Exceptions\LegacyActionFormatException;
 use MagicLink\MagicLink;
 use MagicLink\Security\Serializable\Serializable;
 
@@ -79,11 +79,7 @@ public function run()
         try {
             return $this->callResponse(Serializable::unserialize($this->httpResponse));
         } catch (Exception $e) {
-            return $this->callResponse(unserialize($this->httpResponse, ['allowed_classes' => [
-                RedirectResponse::class,
-                SerializableClosure::class,
-                Signed::class,
-            ]]));
+            throw LegacyActionFormatException::detected($e);
         }
     }
 

src/Controllers/MagicLinkController.php

@@ -3,12 +3,20 @@
 namespace MagicLink\Controllers;
 
 use Illuminate\Routing\Controller;
+use Illuminate\Support\Facades\Log;
+use MagicLink\Exceptions\LegacyActionFormatException;
 use MagicLink\MagicLink;
 
 class MagicLinkController extends Controller
 {
     public function access($token)
     {
-        return MagicLink::getMagicLinkByToken($token)->run();
+        try {
+            return MagicLink::getMagicLinkByToken($token)->run();
+        } catch (LegacyActionFormatException $e) {
+            Log::error('Legacy action format detected for token: '.$token.'. Error: '.$e->getMessage());
+
+            return response()->json(['message' => 'This magic link is no longer valid. Please request a new one.'], 419);
+        }
     }
 }

src/Exceptions/LegacyActionFormatException.php

@@ -0,0 +1,22 @@
+<?php
+
+namespace MagicLink\Exceptions;
+
+use RuntimeException;
+
+class LegacyActionFormatException extends RuntimeException
+{
+    /**
+     * Create a new LegacyActionFormatException instance.
+     *
+     * @param  \Exception|null  $previous
+     */
+    public static function detected($previous = null): LegacyActionFormatException
+    {
+        return new self(
+            'Legacy action format detected. Please run the migration command to update your magic links: php artisan magiclink:migrate',
+            0,
+            $previous
+        );
+    }
+}

src/MagicLink.php

@@ -12,8 +12,8 @@
 use MagicLink\Events\MagicLinkWasCreated;
 use MagicLink\Events\MagicLinkWasDeleted;
 use MagicLink\Events\MagicLinkWasVisited;
+use MagicLink\Exceptions\LegacyActionFormatException;
 use MagicLink\Security\Serializable\ActionSerializable;
-use MagicLink\Security\Serializable\LegacyAllowClasses;
 
 /**
  * @property string $token
@@ -61,16 +61,10 @@ protected static function getTokenLength()
     public function getActionAttribute($value)
     {
         try {
-            $action = ActionSerializable::unserialize($value);
+            return ActionSerializable::unserialize($value);
         } catch (\Exception $e) {
-            $action = $this->legacyGetAction($value);
+            throw LegacyActionFormatException::detected($e);
         }
-
-        if (! $action instanceof ActionAbstract) {
-            throw new \RuntimeException('Invalid action type. Only ActionAbstract instances are allowed.');
-        }
-
-        return $action;
     }
 
     public function setActionAttribute($value)
@@ -82,25 +76,6 @@ public function setActionAttribute($value)
         $this->attributes['action'] = ActionSerializable::serialize($value);
     }
 
-    #[\Deprecated('Please run php artisan magiclink:migrate to migrate legacy actions.', 'v2.24.3')]
-    private function legacyGetAction($value)
-    {
-        $data = $this->getConnection()->getDriverName() === 'pgsql'
-                ? base64_decode($value)
-                : $value;
-
-        if (preg_match('/^O:\d+:"([^"]+)"/', $data, $matches)) {
-            $className = $matches[1];
-            if (is_subclass_of($className, ActionAbstract::class)) {
-                return unserialize($data, [
-                    'allowed_classes' => LegacyAllowClasses::get($className),
-                ]);
-            }
-        }
-
-        throw new \RuntimeException('Invalid legacy action class');
-    }
-
     public function baseUrl(?string $baseUrl): self
     {
         $this->attributes['base_url'] = rtrim($baseUrl, '/').'/';

src/Security/Serializable/LegacyAllowClasses.php

@@ -22,31 +22,23 @@ public function test_controller()
         $magiclinkUrl = $this->generateMagicLink($action);
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('im a controller invoke');
+            ->assertStatus(419);
     }
 
     public function test_download_file()
     {
         $magiclinkUrl = $this->generateMagicLink(new DownloadFileAction('text.txt'));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertHeader(
-                'content-disposition',
-                'attachment; filename=text.txt'
-            );
+            ->assertStatus(419);
     }
 
     public function test_auth()
     {
         $magiclinkUrl = $this->generateMagicLink(new LoginAction(User::first()));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(302)
-            ->assertRedirect('/');
-
-        $this->assertAuthenticatedAs(User::first());
+            ->assertStatus(419);
     }
 
     public function test_response_callable()
@@ -58,17 +50,17 @@ function () {
         ));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('callback called');
+            ->assertStatus(419)
+            ->assertDontSee('callback called');
     }
 
     public function test_view()
     {
         $magiclinkUrl = $this->generateMagicLink(new ViewAction('view'));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('This is a tests view');
+            ->assertStatus(419)
+            ->assertDontSee('This is a tests view');
     }
 
     private function generateMagicLink($action): string

tests/Actions/LegacyTest.php

@@ -22,8 +22,7 @@ public function test_controller()
         [$id, $magiclinkUrl] = $this->generateLegacyMagicLink($action);
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('im a controller invoke');
+            ->assertStatus(419);
 
         $this->artisan('magiclink:migrate', ['--force' => true])
             ->assertSuccessful();
@@ -42,11 +41,7 @@ public function test_download_file()
         [$id, $magiclinkUrl] = $this->generateLegacyMagicLink(new DownloadFileAction('text.txt'));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertHeader(
-                'content-disposition',
-                'attachment; filename=text.txt'
-            );
+            ->assertStatus(419);
 
         $this->artisan('magiclink:migrate', ['--force' => true])
             ->assertSuccessful();
@@ -90,8 +85,7 @@ function () {
         ));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('callback called');
+            ->assertStatus(419);
 
         $this->artisan('magiclink:migrate', ['--force' => true])
             ->assertSuccessful();
@@ -110,8 +104,7 @@ public function test_view()
         [$id, $magiclinkUrl] = $this->generateLegacyMagicLink(new ViewAction('view'));
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('This is a tests view');
+            ->assertStatus(419);
 
         $this->artisan('magiclink:migrate', ['--force' => true])
             ->assertSuccessful();
@@ -132,8 +125,7 @@ public function test_dry_run()
         [$id, $magiclinkUrl] = $this->generateLegacyMagicLink($action);
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('im a controller invoke');
+            ->assertStatus(419);
 
         $actionLegacy = DB::table('magic_links')->where('id', $id)->first()->action;
 
@@ -143,8 +135,7 @@ public function test_dry_run()
         $this->assertEquals($actionLegacy, DB::table('magic_links')->where('id', $id)->first()->action);
 
         $this->get($magiclinkUrl)
-            ->assertStatus(200)
-            ->assertSeeText('im a controller invoke');
+            ->assertStatus(419);
     }
 
     private function generateLegacyMagicLink($action): array