Added test for .env visibility for setup page

app/Http/Controllers/SettingsController.php

@@ -14,6 +14,7 @@ use App\Models\Asset;
 use App\Models\User;
 use App\Notifications\FirstAdminNotification;
 use App\Notifications\MailTest;
+use Illuminate\Http\Client\HttpClientException;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\App;
 use Illuminate\Support\Facades\Storage;
@@ -24,7 +25,9 @@ use Illuminate\Support\Facades\Artisan;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Crypt;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Facades\URL;
 use Illuminate\Support\Facades\Validator;
 
 /**
@@ -89,20 +92,7 @@ class SettingsController extends Controller
         $start_settings['php_version_min'] = true;
 
         // Curl the .env file to make sure it's not accessible via a browser
-        $ch = curl_init($protocol.$host.'/.env');
-        curl_setopt($ch, CURLOPT_HEADER, true);    // we want headers
-        curl_setopt($ch, CURLOPT_NOBODY, true);    // we don't need body
-        curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
-        curl_setopt($ch, CURLOPT_TIMEOUT, 10);
-        $output = curl_exec($ch);
-        $httpcode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
-        curl_close($ch);
-
-        if (404 == $httpcode || 403 == $httpcode || 0 == $httpcode) {
-            $start_settings['env_exposed'] = false;
-        } else {
-            $start_settings['env_exposed'] = true;
-        }
+        $start_settings['env_exposed'] = $this->dotEnvFileIsExposed();
 
         if (App::Environment('production') && (true == config('app.debug'))) {
             $start_settings['debug_exposed'] = true;
@@ -155,6 +145,25 @@ class SettingsController extends Controller
             ->with('section', 'Pre-Flight Check');
     }
 
+    /**
+     * Determine if the .env file accessible via a browser.
+     *
+     * @return bool This method will return true when exceptions (such as curl exception) is thrown.
+     * Check the log files to see more details about the exception.
+     */
+    protected function dotEnvFileIsExposed()
+    {
+        try {
+            return Http::timeout(10)
+                ->accept('*/*')
+                ->get(URL::to('.env'))
+                ->successful();
+        } catch (HttpClientException $e) {
+            Log::debug($e->getMessage());
+            return true;
+        }
+    }
+
     /**
      * Save the first admin user from Setup.
      *
@@ -458,7 +467,6 @@ class SettingsController extends Controller
                 Storage::disk('public')->delete($setting->favicon);
                 $setting->favicon = null;
             }
-
         }
 
         if ($setting->save()) {
@@ -966,8 +974,6 @@ class SettingsController extends Controller
             $setting->ldap_dept = $request->input('ldap_dept');
             $setting->ldap_client_tls_cert   = $request->input('ldap_client_tls_cert');
             $setting->ldap_client_tls_key    = $request->input('ldap_client_tls_key');
-
-
         }
 
         if ($setting->save()) {
@@ -1114,8 +1120,6 @@ class SettingsController extends Controller
 
                     ];
                 }
-
-
             }
         }
 
@@ -1209,7 +1213,6 @@ class SettingsController extends Controller
                     } catch (\Exception $e) {
                         Log::debug($e);
                     }
-
                 } else {
                     return redirect()->route('settings.backups.index')->with('error', trans('admin/settings/message.backup.file_not_found'));
                 }
@@ -1256,15 +1259,10 @@ class SettingsController extends Controller
                 }
 
                 return redirect()->route('settings.backups.index')->withErrors($validator);
-
             }
-
         } else {
             return redirect()->route('settings.backups.index')->with('error', trans('general.feature_disabled'));
         }
-
-
-
     }
 
     /**
@@ -1330,7 +1328,6 @@ class SettingsController extends Controller
                 Auth::logout();
 
                 return redirect()->route('login')->with('success', 'Your system has been restored. Please login again.');
-
             } else {
                 return redirect()->route('settings.backups.index')->with('error', trans('admin/settings/message.backup.file_not_found'));
             }
@@ -1358,7 +1355,6 @@ class SettingsController extends Controller
         }
 
         return redirect()->route('settings.index')->with('error', trans('general.purge_not_allowed'));
-
     }
 
     /**
@@ -1389,7 +1385,6 @@ class SettingsController extends Controller
 
                     return redirect()->route('settings.index')
                         ->with('output', $output)->with('success', trans('admin/settings/message.purge.success'));
-
                 } else {
                     return redirect()->route('settings.purge.index')
                         ->with('error', trans('admin/settings/message.purge.validation_failed'));

tests/Feature/Settings/ShowSetUpPageTest.php

@@ -3,14 +3,27 @@
 namespace Tests\Feature\Settings;
 
 use Illuminate\Database\Events\QueryExecuted;
+use Illuminate\Http\Client\ConnectionException;
+use Illuminate\Http\Client\Request;
+use Illuminate\Http\Client\Response;
+use Illuminate\Log\Events\MessageLogged;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Event;
+use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Facades\URL;
 use Illuminate\Testing\TestResponse;
 use PDOException;
 use Tests\TestCase;
 
 class ShowSetUpPageTest extends TestCase
 {
+    /**
+     * We do not want to make actual http request on every test to check .env file
+     * visibility because that can be really slow especially in some cases where an
+     * actual server is not running.
+     */
+    protected bool $preventStrayRequest = true;
+
     protected function setUp(): void
     {
         parent::setUp();
@@ -20,6 +33,10 @@ class ShowSetUpPageTest extends TestCase
 
     protected function getSetUpPageResponse(): TestResponse
     {
+        if ($this->preventStrayRequest) {
+            Http::fake([URL::to('.env') => Http::response(null, 404)]);
+        }
+
         return $this->get('/setup');
     }
 
@@ -138,4 +155,82 @@ class ShowSetUpPageTest extends TestCase
 
         $this->assertSeeEnvironmentMisconfigurationErrorMessage(false);
     }
+
+    public function testWillCheckDotEnvFileVisibility(): void
+    {
+        $this->getSetUpPageResponse()->assertOk();
+
+        Http::assertSent(function (Request $request) {
+            $this->assertEquals('GET', $request->method());
+            $this->assertEquals(URL::to('.env'), $request->url());
+            return true;
+        });
+    }
+
+    /**
+     * @dataProvider willShowErrorWhenDotEnvFileIsAccessibleViaHttpData
+     */
+    public function testWillShowErrorWhenDotEnvFileIsAccessibleViaHttp(int $statusCode): void
+    {
+        $this->preventStrayRequest = false;
+
+        Http::fake([URL::to('.env') => Http::response(null, $statusCode)]);
+
+        $this->getSetUpPageResponse()->assertOk();
+
+        Http::assertSent(function (Request $request, Response $response) use ($statusCode) {
+            $this->assertEquals($statusCode, $response->status());
+            return true;
+        });
+
+        $this->assertSeeDotEnvFileExposedErrorMessage();
+    }
+
+    public static function willShowErrorWhenDotEnvFileIsAccessibleViaHttpData(): array
+    {
+        return collect([200, 202, 204, 206])
+            ->mapWithKeys(fn (int $code) => ["StatusCode: {$code}" => [$code]])
+            ->all();
+    }
+
+    protected function assertSeeDotEnvFileExposedErrorMessage(bool $shouldSee = true): void
+    {
+        $errorMessage = "We cannot determine if your config file is exposed to the outside world, so you will have to manually verify this. You don't ever want anyone able to see that file. Ever. Ever ever. An exposed <code>.env</code> file can disclose sensitive data about your system and database.";
+        $successMessage = "Sweet. It doesn't look like your <code>.env</code> file is exposed to the outside world. (You should double check this in a browser though. You don't ever want anyone able to see that file. Ever. Ever ever.) <a href=\"../../.env\">Click here to check now</a> (This should return a file not found or forbidden error.)";
+
+        if ($shouldSee) {
+            self::$latestResponse->assertSee($errorMessage, false)->assertDontSee($successMessage, false);
+
+            return;
+        }
+
+        self::$latestResponse->assertSee($successMessage, false)->assertDontSee($errorMessage, false);
+    }
+
+    public function testWillNotShowErrorWhenDotEnvFileIsNotAccessibleViaHttp(): void
+    {
+        $this->getSetUpPageResponse()->assertOk();
+
+        $this->assertSeeDotEnvFileExposedErrorMessage(false);
+    }
+
+    public function testWillShowErrorWhenDotEnvFileVisibilityCheckRequestFails(): void
+    {
+        $this->preventStrayRequest = false;
+
+        Http::fake([URL::to('.env') => fn () => throw new ConnectionException('Some curl error message.')]);
+
+        Event::fake();
+
+        $this->getSetUpPageResponse()->assertOk();
+
+        $this->assertSeeDotEnvFileExposedErrorMessage();
+
+        Event::assertDispatched(function (MessageLogged $event) {
+            $this->assertEquals('debug', $event->level);
+            $this->assertEquals('Some curl error message.', $event->message);
+
+            return true;
+        });
+    }
 }