laravel / pulse

Laravel Pulse is a real-time application performance monitoring tool and dashboard for your Laravel application.
https://pulse.laravel.com
MIT License
1.44k stars 165 forks source link

Livewire update does not apply middlewares set up in the configuration #277

Closed Vinksyunit closed 9 months ago

Vinksyunit commented 9 months ago

Pulse Version

v1.0.0-beta9

Laravel Version

10.40.0

PHP Version

8.3.1

Livewire Version

3.3.5

Database Driver & Version

postgres:16-alpine3.19 (Docker linux amd/64)

Description

Working on a solution with two different user's guards, we notice that the POST /livewire/update requests does not use the intended guard for pulse. Indeed, the auth:other_guard middleware is set up in the Pulse config file, but it seems that the POST livewire/update request does not use the middleware in the pulse config and instead, only the web middleware, thus using the default user guard.

Steps To Reproduce

  1. Set up a second Auth provider (admin_session in this example)

  2. Add another user guard in config/auth.php.

<?php

use App\Models\User;

return [
    'defaults' => [
        'guard' => 'web',
        'passwords' => 'users',
    ],

    'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],
        'admin' => [
            'driver' => 'admin_session',
            'provider' => 'users',
        ],
    ],
   // ...
}
  1. Adapt pulse config (config/pulse.php) to use the intended guard
use Laravel\Pulse\Http\Middleware\Authorize;

return [
    'middleware' => [
        'auth:admin',
        'web',
        Authorize::class,
    ],
]
  1. Connect to both user systems, and notice that the main pulse page uses the admin guard as intended, whereas POST livewire/update uses the default webguard. It can be easily noticed using the viewPulse Gate.
use Illuminate\Support\Facades\Gate;
use App\Models\User;

Gate::define('viewPulse', function (?User $user) {
    if ($user->login !== 'theoneconnectedtotheadmin') {
       throw new \Exception('Wrong one');
    }
    return true;
});
Vinksyunit commented 9 months ago

Just noticed https://github.com/laravel/pulse/issues/269, I will have a look to check if issue is indeed the same later today.

Edit: it seems more an issue about detecting the user for the context and not for authenticating Pulse requests.

timacdonald commented 9 months ago

I'm unable to replicate this problem. Below is what I am using against a fresh laravel application.

I am seeing all the expected logging for both the initial request and subsequent requests. The exception is never thrown.

Can you please create a minimal reproduction for this and we take a closer look?

laravel new bug-report --github="--public"

Service provider

class AppServiceProvider extends ServiceProvider
{
    public function register(): void
    {
        //
    }

    public function boot(): void
    {
        Auth::extend('admin_session', function () {
            Log::info('Using auth via custom guard.');

            return new class implements AuthGuard {
                public function check()
                {
                    return true;
                }

                public function guest()
                {
                    return false;
                }

                public function user()
                {
                    return User::first()->setAttribute('from_custom_guard', true);
                }

                public function id()
                {
                    return $this->user()->id;
                }

                public function validate(array $credentials = [])
                {
                    return true;
                }

                public function hasUser()
                {
                    return true;
                }

                public function setUser(Authenticatable $user)
                {
                    //
                }
            };
        });

        Gate::define('viewPulse', function (?User $user) {
            if ($user->from_custom_guard !== true) {
                throw new Exception('Wrong one!');
            }

            Log::info('Able to view Pulse.');

            return true;
        });
    }
}

Auth Config

'guards' => [
    'web' => [
        'driver' => 'session',
        'provider' => 'users',
    ],
    'admin' => [
        'driver' => 'admin_session',
        'provider' => 'users',
    ],
],

Pulse Config

'middleware' => [
    'auth:admin',
    'web',
    Authorize::class,
],
Vinksyunit commented 9 months ago

Thanks for the quick reply. I had a look with my team this morning and we identified what's missing here to reproduce. We simplified too much the context of the issue (and we are sorry for that 🙏).

The missing step is that we have a custom Authenticate middleware for admins in order to redirect to another route.

<?php 

namespace App\Http\Middleware;

use Illuminate\Auth\Middleware\Authenticate;
use Illuminate\Http\Request;

class AdminAuthenticate extends Authenticate {
    protected function redirectTo(Request $request)
    {
        return '/';
    }
}

So our middleware configuration in pulse.php looks like this :

<?php
use App\Http\Middleware\AdminAuthenticate;

return [
    'middleware' => [
        AdminAuthenticate:class . ':admin',
        'web',
        Authorize::class,
    ],
]

We changed it to this and everything works fine.

<?php
return [
    'middleware' => [
        'auth:admin',
        'web',
        Authorize::class,
    ],
]

As a workaround, we changed the middleware in pulse, so the issue is not as critical for us, but we still think it could be usefull to be able to use custom authentication middlewares in pulse later. Perhaps, something to do with livewire configuration ? I'm not familiar with it.

We will provide you a minimal repo tomorrow to easily reproduce.

timacdonald commented 9 months ago

I've been able to replicate this issue. Looks like an issue with Livewire.

I'm investigating a fix.

timacdonald commented 9 months ago

Opened a fix for this one: https://github.com/laravel/pulse/pull/283

I'll close this and we can follow along in that PR.