spatie / laravel-web-tinker

Tinker in your browser
https://spatie.be/open-source
MIT License
1.05k stars 68 forks source link

User is always null? #32

Closed repat closed 5 years ago

repat commented 5 years ago

image

If I remove = null I get a 403. Am I doing something wrong here or is this part broken?

>>> env('WEB_TINKER_ENABLED', false)
=> true
edgrosvenor commented 5 years ago

I'm seeing the same thing.

joaolisboa commented 5 years ago

Same issue. The gate doesn't seem to work at all.

rubenvanassche commented 5 years ago

I guess you're not logged in, since you're trying to get the email address of a non existing user. Just tested it, and it seems that nothing is wrong with the package, since this works:

Auth::login(factory(User::class)->create([
    'email' => 'ruben@spatie.be'
]));

Gate::define('viewWebTinker', function ($user = null) {
    return $user->email === 'ruben@spatie.be';
});

Will close this for now.

edgrosvenor commented 5 years ago

I could also write a test that would pass. It doesn’t mean that it actually works in real life. I assure you I am logged in.

repat commented 5 years ago

I'm also sure I'm logged in as I clicked on it in /admin ;-) Did you also move yours @edgrosvenor ? Maybe that's part of the issue for whatever reason 🤔

joaolisboa commented 5 years ago

I'm also sure I'm logged in and have the same exact behavior. With $user = null it comes as null, with $user only I get a 403.

edgrosvenor commented 5 years ago

@repat I didn't change the path. Something is definitely broken.

I'll fork it and figure it out this weekend. As good as testbench is, the "it works fine in my tests but doesn't work when pulled into an actual Laravel app" thing isn't a rare problem.

edgrosvenor commented 5 years ago

I think this might be a Laravel bug actually. It seems that Laravel isn't passing the user to the Gate closure as documented. I wonder if it has to do with the fact that this package explicitly defines its own middleware and does not also use the web middleware. I vaguely remember having a problem like this with another package when I upgraded a site to Laravel 6 and I worked around it by adding the web middleware to the route along with the package's own middleware. I could be wrong and don't have time to dig into it today but I'll experiment over the weekend.

edgrosvenor commented 5 years ago

Yup. That's definitely it. If I use the same gate check in a route that also has the web middleware the user model exists as expected. So I guess maybe we need to ask @rubenvanassche if he and the other Spatie developers think this should be handled via Laravel or here.

joaolisboa commented 5 years ago

If it defines it's own middleware shouldn't that be documented, since it's not using the 'default' web middleware?

edgrosvenor commented 5 years ago

Not necessarily. The Laravel documentation doesn't say that the user will only be provided to the Gate closure if the web middleware is used, though that does seem to be the behavior. I'm not sure who's actually at fault here.

edgrosvenor commented 5 years ago

In the mean time, this will get anyone who's stuck past the problem. Do this in your web.php route file:

use Spatie\WebTinker\Http\Controllers\WebTinkerController;
use Spatie\WebTinker\Http\Middleware\Authorize;

Route::prefix('web_tinker')->middleware(['web', Authorize::class])->group(function () {
    Route::get('/', [WebTinkerController::class, 'index']);
    Route::post('/', [WebTinkerController::class, 'execute']);
});

Obviously use anything you like in the prefix as long as it does not match the path set in the web tinker config file.

rubenvanassche commented 5 years ago

Hi @edgrosvenor,

thanks for your extensive research into this problem! Indeed adding the web middleware group will fix this problem but actually only the StartSession and EncryptsCookies middlewares from the web group are required to solve this problem.

So I've added them(c693f3f951ae9f8d33dc7cd27ea511ad59b0880b) and tagged a new release!

Thanks guys!