[3.x] Ensure device has not been logged out (#467)

* wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * Ensure device has not been logged out This adds a middleware to check that the password has in session is the same as the current users password. This fixes a security issue where an attacker can keep sending requests to an API using the sanctum auth after the password has been changed. Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * Apply fixes from StyleCI * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * Update src/Http/Middleware/EnsureDeviceHasNotBeenLoggedOut.php Co-authored-by: Patrick O'Meara <pat@maintainly.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * Apply fixes from StyleCI * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * Apply suggestions from code review Co-authored-by: Patrick O'Meara <pat@maintainly.com> * Apply fixes from StyleCI * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * Update src/Http/Middleware/AuthenticateSession.php * wip Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> * formatting * formatting --------- Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com> Co-authored-by: Patrick O'Meara <pat@fixd.io> Co-authored-by: StyleCI Bot <bot@styleci.io> Co-authored-by: Patrick O'Meara <pat@maintainly.com> Co-authored-by: Taylor Otwell <taylor@laravel.com>
laravel · Aug 30, 2023 · d64ce69 · d64ce69
1 parent a24affa
commit d64ce69
Show file tree

Hide file tree

Showing 6 changed files with 324 additions and 0 deletions.
diff --git a/config/sanctum.php b/config/sanctum.php
@@ -77,6 +77,7 @@
     'middleware' => [
         'verify_csrf_token' => App\Http\Middleware\VerifyCsrfToken::class,
         'encrypt_cookies' => App\Http\Middleware\EncryptCookies::class,
+        'authenticate_session' => \Laravel\Sanctum\Http\Middleware\AuthenticateSession::class,
     ],
 
 ];
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -14,6 +14,7 @@
     <testsuites>
         <testsuite name="Sanctum Test Suite">
             <directory suffix=".php">./tests/Unit</directory>
+            <directory suffix=".php">./tests/Controller</directory>
             <directory suffix=".php">./tests/Feature</directory>
         </testsuite>
     </testsuites>

diff --git a/src/Http/Middleware/AuthenticateSession.php b/src/Http/Middleware/AuthenticateSession.php
@@ -0,0 +1,90 @@
+<?php
+
+namespace Laravel\Sanctum\Http\Middleware;
+
+use Closure;
+use Illuminate\Auth\AuthenticationException;
+use Illuminate\Auth\SessionGuard;
+use Illuminate\Contracts\Auth\Factory as AuthFactory;
+use Illuminate\Http\Request;
+use Illuminate\Support\Arr;
+use Illuminate\Support\Collection;
+use Symfony\Component\HttpFoundation\Response;
+
+class AuthenticateSession
+{
+    /**
+     * The authentication factory implementation.
+     *
+     * @var \Illuminate\Contracts\Auth\Factory
+     */
+    protected $auth;
+
+    /**
+     * Create a new middleware instance.
+     *
+     * @param  \Illuminate\Contracts\Auth\Factory  $auth
+     * @return void
+     */
+    public function __construct(AuthFactory $auth)
+    {
+        $this->auth = $auth;
+    }
+
+    /**
+     * Handle an incoming request.
+     *
+     * @param  \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response)  $next
+     *
+     * @throws \Illuminate\Auth\AuthenticationException
+     */
+    public function handle(Request $request, Closure $next): Response
+    {
+        if (! $request->hasSession() || ! $request->user()) {
+            return $next($request);
+        }
+
+        $guards = Collection::make(Arr::wrap(config('sanctum.guard')))
+            ->mapWithKeys(fn ($guard) => [$guard => $this->auth->guard($guard)])
+            ->filter(fn ($guard) => $guard instanceof SessionGuard);
+
+        $shouldLogout = $guards->filter(
+            fn ($guard, $driver) => $request->session()->has('password_hash_'.$driver)
+        )->filter(
+            fn ($guard, $driver) => $request->session()->get('password_hash_'.$driver) !==
+                                    $request->user()->getAuthPassword()
+        );
+
+        if ($shouldLogout->isNotEmpty()) {
+            $shouldLogout->each->logoutCurrentDevice();
+
+            $request->session()->flush();
+
+            throw new AuthenticationException('Unauthenticated.', [...$shouldLogout->keys()->all(), 'sanctum']);
+        }
+
+        return tap($next($request), function () use ($request, $guards) {
+            if (! is_null($request->user())) {
+                $this->storePasswordHashInSession($request, $guards->keys()->first());
+            }
+        });
+    }
+
+    /**
+     * Store the user's current password hash in the session.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @param  string  $guard
+     * @return void
+     */
+    protected function storePasswordHashInSession($request, string $guard)
+    {
+        if (! $request->user()) {
+            return;
+        }
+
+        $request->session()->put([
+            "password_hash_{$guard}" => $request->user()->getAuthPassword(),
+        ]);
+    }
+}
diff --git a/src/Http/Middleware/EnsureFrontendRequestsAreStateful.php b/src/Http/Middleware/EnsureFrontendRequestsAreStateful.php
@@ -52,6 +52,7 @@ protected function frontendMiddleware()
             \Illuminate\Session\Middleware\StartSession::class,
             config('sanctum.middleware.validate_csrf_token'),
             config('sanctum.middleware.verify_csrf_token', \Illuminate\Foundation\Http\Middleware\VerifyCsrfToken::class),
+            config('sanctum.middleware.authenticate_session'),
         ])));
 
         array_unshift($middleware, function ($request, $next) {

diff --git a/tests/Controller/AuthenticateRequestsTest.php b/tests/Controller/AuthenticateRequestsTest.php
@@ -0,0 +1,80 @@
+<?php
+
+namespace Laravel\Sanctum\Tests\Controller;
+
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Http\Request;
+use Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful;
+use Laravel\Sanctum\Sanctum;
+use Orchestra\Testbench\Concerns\WithWorkbench;
+use Orchestra\Testbench\TestCase;
+use Workbench\App\Models\User;
+use Workbench\Database\Factories\PersonalAccessTokenFactory;
+use Workbench\Database\Factories\UserFactory;
+
+class AuthenticateRequestsTest extends TestCase
+{
+    use RefreshDatabase, WithWorkbench;
+
+    protected function defineEnvironment($app)
+    {
+        $app['config']->set([
+            'app.key' => 'AckfSECXIvnK5r28GVIWUAxmbBSjTsmF',
+            'auth.guards.sanctum.provider' => 'users',
+            'auth.providers.users.model' => User::class,
+            'database.default' => 'testing',
+            'sanctum.middleware.encrypt_cookies' => \Illuminate\Cookie\Middleware\EncryptCookies::class,
+            'sanctum.middleware.verify_csrf_token' => \Illuminate\Foundation\Http\Middleware\VerifyCsrfToken::class,
+        ]);
+    }
+
+    protected function defineRoutes($router)
+    {
+        $apiMiddleware = [EnsureFrontendRequestsAreStateful::class, 'api', 'auth:sanctum'];
+
+        $router->get('/sanctum/api/user', function (Request $request) {
+            abort_if(is_null($request->user()), 401);
+
+            return $request->user()->email;
+        })->middleware($apiMiddleware);
+
+        $router->get('/sanctum/web/user', function (Request $request) {
+            abort_if(is_null($request->user()), 401);
+
+            return $request->user()->email;
+        })->middleware($apiMiddleware);
+    }
+
+    public function test_can_authorize_valid_user_using_authorization_header()
+    {
+        PersonalAccessTokenFactory::new()->for(
+            $user = UserFactory::new()->create(), 'tokenable'
+        )->create();
+
+        $this->getJson('/sanctum/api/user', ['Authorization' => 'Bearer test'])
+            ->assertOk()
+            ->assertSee($user->email);
+    }
+
+    /**
+     * @dataProvider sanctumGuardsDataProvider
+     */
+    public function test_can_authorize_valid_user_using_sanctum_acting_as($guard)
+    {
+        PersonalAccessTokenFactory::new()->for(
+            $user = UserFactory::new()->create(), 'tokenable'
+        )->create();
+
+        Sanctum::actingAs($user, [], $guard);
+
+        $this->getJson('/sanctum/api/user')
+            ->assertOk()
+            ->assertSee($user->email);
+    }
+
+    public static function sanctumGuardsDataProvider()
+    {
+        yield [null];
+        yield ['web'];
+    }
+}
diff --git a/tests/Controller/FrontendRequestsAreStatefulTest.php b/tests/Controller/FrontendRequestsAreStatefulTest.php
@@ -0,0 +1,151 @@
+<?php
+
+namespace Laravel\Sanctum\Tests\Controller;
+
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Http\Request;
+use Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful;
+use Laravel\Sanctum\Sanctum;
+use Orchestra\Testbench\Concerns\WithWorkbench;
+use Orchestra\Testbench\TestCase;
+use Workbench\App\Models\User;
+use Workbench\Database\Factories\UserFactory;
+
+class FrontendRequestsAreStatefulTest extends TestCase
+{
+    use RefreshDatabase, WithWorkbench;
+
+    protected function defineEnvironment($app)
+    {
+        $app['config']->set([
+            'app.key' => 'AckfSECXIvnK5r28GVIWUAxmbBSjTsmF',
+            'auth.guards.sanctum.provider' => 'users',
+            'auth.providers.users.model' => User::class,
+            'database.default' => 'testing',
+            'sanctum.middleware.encrypt_cookies' => \Illuminate\Cookie\Middleware\EncryptCookies::class,
+            'sanctum.middleware.verify_csrf_token' => \Illuminate\Foundation\Http\Middleware\VerifyCsrfToken::class,
+        ]);
+    }
+
+    protected function defineRoutes($router)
+    {
+        $webMiddleware = ['web', 'auth.session'];
+        $apiMiddleware = [EnsureFrontendRequestsAreStateful::class, 'api', 'auth:sanctum'];
+
+        $router->get('/sanctum/api/user', function (Request $request) {
+            abort_if(is_null($request->user()), 401);
+
+            return $request->user()->email;
+        })->middleware($apiMiddleware);
+
+        $router->post('/sanctum/api/password', function (Request $request) {
+            abort_if(is_null($request->user()), 401);
+
+            $request->user()->update(['password' => bcrypt('laravel')]);
+
+            return $request->user()->email;
+        })->middleware($apiMiddleware);
+
+        $router->get('/sanctum/web/user', function (Request $request) {
+            abort_if(is_null($request->user()), 401);
+
+            return $request->user()->email;
+        })->middleware($apiMiddleware);
+
+        $router->get('web/user', function (Request $request) {
+            abort_if(is_null($request->user()), 401);
+
+            return $request->user()->email;
+        })->middleware($webMiddleware);
+    }
+
+    public function test_middleware_keeps_session_logged_in_when_sanctum_request_changes_password()
+    {
+        $user = UserFactory::new()->create();
+
+        $this->actingAs($user)
+            ->getJson('/web/user', [
+                'origin' => config('app.url'),
+            ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $this->getJson('/sanctum/api/user', [
+            'origin' => config('app.url'),
+        ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $this->postJson('/sanctum/api/password', [], [
+            'origin' => config('app.url'),
+        ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $this->getJson('/sanctum/api/user', [
+            'origin' => config('app.url'),
+        ])
+            ->assertOk()
+            ->assertSee($user->email);
+    }
+
+    /**
+     * @dataProvider sanctumGuardsDataProvider
+     */
+    public function test_middleware_can_deauthorize_valid_user_using_acting_as_after_password_change_from_sanctum_guard($guard)
+    {
+        $user = UserFactory::new()->create();
+
+        Sanctum::actingAs($user, [], $guard);
+
+        $this->getJson('/web/user', [
+            'origin' => config('app.url'),
+        ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $this->getJson('/sanctum/web/user', [
+            'origin' => config('app.url'),
+        ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $user->password = bcrypt('laravel');
+        $user->save();
+
+        $this->getJson('/sanctum/web/user', [
+            'origin' => config('app.url'),
+        ])->assertStatus(401);
+    }
+
+    public function test_middleware_can_deauthorize_valid_user_using_acting_as_after_password_change_coming_from_web_guard()
+    {
+        $user = UserFactory::new()->create();
+
+        $this->actingAs($user)
+            ->getJson('/web/user', [
+                'origin' => config('app.url'),
+            ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $this->getJson('/sanctum/web/user', [
+            'origin' => config('app.url'),
+        ])
+            ->assertOk()
+            ->assertSee($user->email);
+
+        $user->password = bcrypt('laravel');
+        $user->save();
+
+        $this->getJson('/sanctum/web/user', [
+            'origin' => config('app.url'),
+        ])->assertStatus(401);
+    }
+
+    public static function sanctumGuardsDataProvider()
+    {
+        yield [null];
+        yield ['web'];
+    }
+}