From 588d1fcc0db8a1f612738f2f4e5ee0e5f23aa52f Mon Sep 17 00:00:00 2001
From: lukinovec <lukinovec@gmail.com>
Date: Thu, 15 May 2025 14:54:04 +0200
Subject: [PATCH] [4.x] Make TableRLSManager skip foreign keys with 'no-rls'
 comment right away (#1352)

* When a foreign key has no-rls comment (or no comment when scopeByDefault is false), skip path generation earlier

* Fix column definitions
---
 src/RLS/PolicyManagers/TableRLSManager.php | 16 +++++++---------
 tests/RLS/TableManagerTest.php             | 19 ++++++++++++++++---
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/RLS/PolicyManagers/TableRLSManager.php b/src/RLS/PolicyManagers/TableRLSManager.php
index 61c62f94..8e941b31 100644
--- a/src/RLS/PolicyManagers/TableRLSManager.php
+++ b/src/RLS/PolicyManagers/TableRLSManager.php
@@ -108,6 +108,12 @@ class TableRLSManager implements RLSPolicyManager
 
     protected function generatePaths(string $table, array $foreign, array &$paths, array $currentPath = []): void
     {
+        // If the foreign key has a comment of 'no-rls', we skip it
+        // Also skip the foreign key if implicit scoping is off and the foreign key has no comment
+        if ($foreign['comment'] === 'no-rls' || (! static::$scopeByDefault && $foreign['comment'] === null)) {
+            return;
+        }
+
         if (in_array($foreign['foreignTable'], array_column($currentPath, 'foreignTable'))) {
             throw new RecursiveRelationshipException;
         }
@@ -115,15 +121,7 @@ class TableRLSManager implements RLSPolicyManager
         $currentPath[] = $foreign;
 
         if ($foreign['foreignTable'] === tenancy()->model()->getTable()) {
-            $comments = array_column($currentPath, 'comment');
-            $pathCanUseRls = static::$scopeByDefault ?
-                ! in_array('no-rls', $comments) :
-                ! in_array('no-rls', $comments) && ! in_array(null, $comments);
-
-            if ($pathCanUseRls) {
-                // If the foreign table is the tenants table, add the current path to $paths
-                $paths[] = $currentPath;
-            }
+            $paths[] = $currentPath;
         } else {
             // If not, recursively generate paths for the foreign table
             foreach ($this->database->getSchemaBuilder()->getForeignKeys($foreign['foreignTable']) as $nextConstraint) {
diff --git a/tests/RLS/TableManagerTest.php b/tests/RLS/TableManagerTest.php
index 4a8ac058..fd9c6f44 100644
--- a/tests/RLS/TableManagerTest.php
+++ b/tests/RLS/TableManagerTest.php
@@ -503,7 +503,7 @@ test('table rls manager generates relationship trees with tables related to the
 
     // Add non-nullable comment_id foreign key
     Schema::table('ratings', function (Blueprint $table) {
-        $table->foreignId('comment_id')->onUpdate('cascade')->onDelete('cascade')->comment('rls')->constrained('comments');
+        $table->foreignId('comment_id')->comment('rls')->constrained('comments')->onUpdate('cascade')->onDelete('cascade');
     });
 
     // Non-nullable paths are preferred over nullable paths
@@ -640,16 +640,29 @@ test('table rls manager generates queries correctly', function() {
 test('table manager throws an exception when encountering a recursive relationship', function() {
     Schema::create('recursive_posts', function (Blueprint $table) {
         $table->id();
-        $table->foreignId('highlighted_comment_id')->constrained('comments')->nullable()->comment('rls');
+        $table->foreignId('highlighted_comment_id')->nullable()->comment('rls')->constrained('comments');
     });
 
     Schema::table('comments', function (Blueprint $table) {
-        $table->foreignId('recursive_post_id')->constrained('recursive_posts')->comment('rls');
+        $table->foreignId('recursive_post_id')->comment('rls')->constrained('recursive_posts');
     });
 
     expect(fn () => app(TableRLSManager::class)->generateTrees())->toThrow(RecursiveRelationshipException::class);
 });
 
+test('table manager ignores recursive relationship if the foreign key responsible for the recursion has no-rls comment', function() {
+    Schema::create('recursive_posts', function (Blueprint $table) {
+        $table->id();
+        $table->foreignId('highlighted_comment_id')->nullable()->comment('no-rls')->constrained('comments');
+    });
+
+    Schema::table('comments', function (Blueprint $table) {
+        $table->foreignId('recursive_post_id')->comment('rls')->constrained('recursive_posts');
+    });
+
+    expect(fn () => app(TableRLSManager::class)->generateTrees())->not()->toThrow(RecursiveRelationshipException::class);
+});
+
 class Post extends Model
 {
     protected $guarded = [];