From 2d500f97808380dfaae202121138beb48457f8a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuel=20=C5=A0tancl?= <samuel.stancl@gmail.com>
Date: Fri, 18 Aug 2023 20:21:00 +0200
Subject: [PATCH] Fix #1112 - throw an exception if DATABASE_URL is set (#9)

* fix #1112 - throw an exception when DATABASE_URL is defined, minor test changes

* Fix code style (php-cs-fixer)

* fix typo

---------

Co-authored-by: PHP CS Fixer <phpcsfixer@example.com>
---
 docker-compose.yml                             |  6 ++++++
 .../DatabaseTenancyBootstrapper.php            |  8 ++++++++
 src/Database/DatabaseConfig.php                |  1 -
 tests/BootstrapperTest.php                     | 18 ++++++++++++++++++
 tests/QueueTest.php                            |  1 +
 5 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/docker-compose.yml b/docker-compose.yml
index 465b36cd..e7d14a80 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -40,6 +40,8 @@ services:
             test: ["CMD", "mysqladmin" ,"ping", "-h", "localhost"]
             timeout: 10s
             retries: 10
+        tmpfs:
+            - /var/lib/mysql
     mysql2:
         image: mysql:5.7
         environment:
@@ -51,6 +53,8 @@ services:
             test: ["CMD", "mysqladmin" ,"ping", "-h", "localhost"]
             timeout: 10s
             retries: 10
+        tmpfs:
+            - /var/lib/mysql
     postgres:
         image: postgres:11
         environment:
@@ -62,6 +66,8 @@ services:
             interval: 10s
             timeout: 5s
             retries: 5
+        tmpfs:
+            - /var/lib/postgresql/data
     redis:
         image: redis:alpine
         healthcheck:
diff --git a/src/Bootstrappers/DatabaseTenancyBootstrapper.php b/src/Bootstrappers/DatabaseTenancyBootstrapper.php
index f058dc43..a79ad275 100644
--- a/src/Bootstrappers/DatabaseTenancyBootstrapper.php
+++ b/src/Bootstrappers/DatabaseTenancyBootstrapper.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Bootstrappers;
 
+use Exception;
 use Stancl\Tenancy\Contracts\TenancyBootstrapper;
 use Stancl\Tenancy\Contracts\Tenant;
 use Stancl\Tenancy\Database\Contracts\TenantWithDatabase;
@@ -23,6 +24,13 @@ class DatabaseTenancyBootstrapper implements TenancyBootstrapper
     public function bootstrap(Tenant $tenant): void
     {
         /** @var TenantWithDatabase $tenant */
+        if (data_get($tenant->database()->getTemplateConnection(), 'url')) {
+            // The package works with individual parts of the database connection config, so DATABASE_URL is not supported.
+            // When DATABASE_URL is set, this bootstrapper can silently fail i.e. keep using the template connection's database URL
+            // which takes precedence over individual segments of the connection config. This issue can be hard to debug as it can be
+            // production-specific. Therefore, we throw an exception (that effectively blocks all tenant pages) to prevent incorrect DB use.
+            throw new Exception('The template connection must NOT have URL defined. Specify the connection using individual parts instead of a database URL.');
+        }
 
         // Better debugging, but breaks cached lookup in prod
         if (app()->environment('local') || app()->environment('testing')) { // todo@docs mention this change in v4 upgrade guide https://github.com/archtechx/tenancy/pull/945#issuecomment-1268206149
diff --git a/src/Database/DatabaseConfig.php b/src/Database/DatabaseConfig.php
index 52cb464c..41e14a81 100644
--- a/src/Database/DatabaseConfig.php
+++ b/src/Database/DatabaseConfig.php
@@ -5,7 +5,6 @@ declare(strict_types=1);
 namespace Stancl\Tenancy\Database;
 
 use Closure;
-use Illuminate\Database;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Hash;
diff --git a/tests/BootstrapperTest.php b/tests/BootstrapperTest.php
index 4cab7ddf..f4e20874 100644
--- a/tests/BootstrapperTest.php
+++ b/tests/BootstrapperTest.php
@@ -631,6 +631,24 @@ test('fortify route tenancy bootstrapper updates fortify config correctly', func
     expect(config('fortify.redirects'))->toBe($originalFortifyRedirects);
 });
 
+test('database tenancy bootstrapper throws an exception if DATABASE_URL is set', function (string|null $databaseUrl) {
+    if ($databaseUrl) {
+        config(['database.connections.central.url' => $databaseUrl]);
+
+        pest()->expectException(Exception::class);
+    }
+
+    config(['tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class]]);
+
+    $tenant1 = Tenant::create();
+
+    pest()->artisan('tenants:migrate');
+
+    tenancy()->initialize($tenant1);
+
+    expect(true)->toBe(true);
+})->with(['abc.us-east-1.rds.amazonaws.com', null]);
+
 function getDiskPrefix(string $disk): string
 {
     /** @var FilesystemAdapter $disk */
diff --git a/tests/QueueTest.php b/tests/QueueTest.php
index f88b3934..1c55eb5c 100644
--- a/tests/QueueTest.php
+++ b/tests/QueueTest.php
@@ -159,6 +159,7 @@ test('tenancy is initialized when retrying jobs', function (bool $shouldEndTenan
     });
 })->with([true, false]);
 
+// todo0 this test appears to be affected by race conditions/similar
 test('the tenant used by the job doesnt change when the current tenant changes', function () {
     withTenantDatabases();