Moved force https logic to a middleware; Changed default for config session.secure

korridor · korridor · commit d924fa74ec0e · 2025-02-08T10:40:15.000-05:00
diff --git a/.env.ci b/.env.ci
@@ -5,7 +5,6 @@ APP_DEBUG=true
 APP_URL=http://localhost
 APP_FORCE_HTTPS=false
 APP_ENABLE_REGISTRATION=true
-SESSION_SECURE_COOKIE=false
 
 # Logging
 LOG_CHANNEL=stack
diff --git a/.env.production b/.env.production
@@ -5,7 +5,6 @@ VITE_APP_NAME=solidtime
 APP_ENV=production
 APP_DEBUG=false
 APP_FORCE_HTTPS=true
-SESSION_SECURE_COOKIE=true
 OCTANE_SERVER=frankenphp
 PAGINATION_PER_PAGE_DEFAULT=500
 
diff --git a/app/Filament/Resources/UserResource.php b/app/Filament/Resources/UserResource.php
@@ -42,6 +42,7 @@ public static function form(Form $form): Form
     {
         /** @var User|null $record */
         $record = $form->getRecord();
+
         return $form
             ->columns(1)
             ->schema([
diff --git a/app/Http/Controllers/Web/HealthCheckController.php b/app/Http/Controllers/Web/HealthCheckController.php
@@ -64,6 +64,7 @@ public function debug(Request $request): JsonResponse
             $response['app_env'] = app()->environment();
             $response['app_timezone'] = config('app.timezone');
             $response['app_force_https'] = config('app.force_https');
+            $response['session_secure'] = config('session.secure');
             $response['trusted_proxies'] = config('trustedproxy.proxies');
             $headers = $request->headers->all();
             if (isset($headers['cookie'])) {
diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
@@ -18,7 +18,7 @@ class Kernel extends HttpKernel
      * @var array<int, class-string|string>
      */
     protected $middleware = [
-        // \App\Http\Middleware\TrustHosts::class,
+        \App\Http\Middleware\ForceHttps::class,
         \App\Http\Middleware\TrustProxies::class,
         \Illuminate\Http\Middleware\HandleCors::class,
         \App\Http\Middleware\PreventRequestsDuringMaintenance::class,
diff --git a/app/Http/Middleware/ForceHttps.php b/app/Http/Middleware/ForceHttps.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Http\Middleware;
+
+use Closure;
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\URL;
+use Symfony\Component\HttpFoundation\Response;
+
+class ForceHttps
+{
+    /**
+     * Handle an incoming request.
+     *
+     * @param  \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response)  $next
+     */
+    public function handle(Request $request, Closure $next, string ...$guards): Response
+    {
+        if (config('app.force_https', false)) {
+            URL::forceScheme('https');
+            $request->server->set('HTTPS', 'on');
+            $request->headers->set('X-Forwarded-Proto', 'https');
+        }
+
+        return $next($request);
+    }
+}
diff --git a/app/Http/Middleware/TrustHosts.php b/app/Http/Middleware/TrustHosts.php
diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php
@@ -29,7 +29,6 @@
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Foundation\Application;
 use Illuminate\Support\Facades\Route;
-use Illuminate\Support\Facades\URL;
 use Illuminate\Support\ServiceProvider;
 
 class AppServiceProvider extends ServiceProvider
@@ -90,12 +89,6 @@ public function boot(): void
             );
         });
 
-        if (config('app.force_https', false)) {
-            URL::forceScheme('https');
-            request()->server->set('HTTPS', 'on');
-            request()->headers->set('X-Forwarded-Proto', 'https');
-        }
-
         $this->app->scoped(PermissionStore::class, function (Application $app): PermissionStore {
             return new PermissionStore;
         });
diff --git a/config/session.php b/config/session.php
@@ -168,7 +168,7 @@
     |
     */
 
-    'secure' => env('SESSION_SECURE_COOKIE'),
+    'secure' => env('SESSION_SECURE_COOKIE', env('APP_FORCE_HTTPS')),
 
     /*
     |--------------------------------------------------------------------------
diff --git a/phpunit.xml b/phpunit.xml
@@ -29,6 +29,8 @@
     </source>
     <php>
         <env name="APP_ENV" value="testing"/>
+        <env name="APP_FORCE_HTTPS" value="false"/>
+        <env name="TRUSTED_PROXIES" value="0.0.0.0/0,2000:0:0:0:0:0:0:0/3"/>
         <env name="BCRYPT_ROUNDS" value="4"/>
         <env name="CACHE_DRIVER" value="array"/>
         <env name="DB_CONNECTION" value="pgsql_test"/>
diff --git a/tests/Unit/Endpoint/Web/HealthCheckEndpointTest.php b/tests/Unit/Endpoint/Web/HealthCheckEndpointTest.php
@@ -82,6 +82,7 @@ public function test_debug_endpoint_returns_more_information_if_debug_mode_is_en
             'secure',
             'timestamp',
             'timezone',
+            'session_secure',
             'trusted_proxies',
             'url',
         ]);
diff --git a/tests/Unit/Endpoint/Web/HomeEndpointTest.php b/tests/Unit/Endpoint/Web/HomeEndpointTest.php
@@ -5,11 +5,15 @@
 namespace Tests\Unit\Endpoint\Web;
 
 use App\Http\Controllers\Web\HomeController;
+use App\Http\Middleware\Authenticate;
+use App\Http\Middleware\RedirectIfAuthenticated;
 use App\Models\User;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\UsesClass;
 
 #[CoversClass(HomeController::class)]
+#[CoversClass(Authenticate::class)]
+#[CoversClass(RedirectIfAuthenticated::class)]
 #[UsesClass(HomeController::class)]
 class HomeEndpointTest extends EndpointTestAbstract
 {
@@ -36,4 +40,17 @@ public function test_index_redirects_to_login_if_user_is_not_logged_in(): void
         // Assert
         $response->assertRedirect('/login');
     }
+
+    public function test_login_redirects_to_dashboard_if_user_is_logged_in(): void
+    {
+        // Arrange
+        $user = User::factory()->withPersonalOrganization()->create();
+        $this->actingAs($user);
+
+        // Act
+        $response = $this->get('/login');
+
+        // Assert
+        $response->assertRedirect('/dashboard');
+    }
 }
diff --git a/tests/Unit/Middleware/EnsureEmailIsVerifiedMiddlewareTest.php b/tests/Unit/Middleware/EnsureEmailIsVerifiedMiddlewareTest.php
@@ -47,6 +47,20 @@ public function test_users_with_unverified_email_are_redirected_to_verification_
         $response->assertRedirect(route('verification.notice'));
     }
 
+    public function test_users_with_unverified_email_get_error_if_the_request_is_json(): void
+    {
+        // Arrange
+        $user = User::factory()->unverified()->create();
+        $route = $this->createTestRoute();
+        $this->actingAs($user);
+
+        // Act
+        $response = $this->getJson($route);
+
+        // Assert
+        $response->assertForbidden();
+    }
+
     public function test_users_with_verified_email_can_access_route(): void
     {
         // Arrange
diff --git a/tests/Unit/Middleware/ForceHttpsMiddlewareTest.php b/tests/Unit/Middleware/ForceHttpsMiddlewareTest.php
@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\Middleware;
+
+use App\Http\Middleware\ForceHttps;
+use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Route;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+
+#[CoversClass(ForceHttps::class)]
+#[UsesClass(ForceHttps::class)]
+class ForceHttpsMiddlewareTest extends MiddlewareTestAbstract
+{
+    private function createTestRoute(): string
+    {
+        return Route::get('/test-route', function () {
+            return [
+                'is_secure' => request()->secure(),
+            ];
+        })->middleware(ForceHttps::class)->uri;
+    }
+
+    public function test_if_config_app_force_https_is_true_then_the_request_will_be_modified_to_make_the_app_think_it_was_a_https_request(): void
+    {
+        // Arrange
+        Config::set('app.force_https', true);
+        $route = $this->createTestRoute();
+
+        // Act
+        $response = $this->get($route);
+
+        // Assert
+        $response->assertSuccessful();
+        $response->assertJson(['is_secure' => true]);
+    }
+
+    public function test_if_config_app_force_https_is_true_then_the_request_will_be_modified_to_make_the_app_think_it_was_a_https_request_even_if_a_load_balancer_says_it_was_a_http_request(): void
+    {
+        // Arrange
+        Config::set('app.force_https', true);
+        $route = $this->createTestRoute();
+
+        // Act
+        $response = $this->get($route, ['X-Forwarded-Proto' => 'http']);
+
+        // Assert
+        $response->assertSuccessful();
+        $response->assertJson(['is_secure' => true]);
+    }
+
+    public function test_if_config_app_force_https_is_false_then_the_request_will_not_be_modified_to_make_the_app_think_it_was_a_https_request(): void
+    {
+        // Arrange
+        Config::set('app.force_https', false);
+        $route = $this->createTestRoute();
+
+        // Act
+        $response = $this->get($route);
+
+        // Assert
+        $response->assertSuccessful();
+        $response->assertJson(['is_secure' => false]);
+    }
+
+    public function test_if_config_app_force_https_is_false_then_the_request_will_not_be_modified_but_the_request_can_still_be_https(): void
+    {
+        // Arrange
+        Config::set('app.force_https', false);
+        $route = $this->createTestRoute();
+
+        // Act
+        $response = $this->get($route, ['X-Forwarded-Proto' => 'https']);
+
+        // Assert
+        $response->assertSuccessful();
+        $response->assertJson(['is_secure' => true]);
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,7 @@ public static function form(Form $form): Form`
`42`	`42`	`{`
`43`	`43`	`/** @var User\|null $record */`
`44`	`44`	`$record = $form->getRecord();`
	`45`	`+`
`45`	`46`	`return $form`
`46`	`47`	`->columns(1)`
`47`	`48`	`->schema([`
Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@`
`168`	`168`	`\|`
`169`	`169`	`*/`
`170`	`170`
`171`		`- 'secure' => env('SESSION_SECURE_COOKIE'),`
	`171`	`+ 'secure' => env('SESSION_SECURE_COOKIE', env('APP_FORCE_HTTPS')),`
`172`	`172`
`173`	`173`	`/*`
`174`	`174`	`\|--------------------------------------------------------------------------`