Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gate/Policy breaks when using octane+telescope #1122

Closed
Gab-Menezes opened this issue Sep 6, 2021 · 5 comments
Closed

Gate/Policy breaks when using octane+telescope #1122

Gab-Menezes opened this issue Sep 6, 2021 · 5 comments
Assignees

Comments

@Gab-Menezes
Copy link

Gab-Menezes commented Sep 6, 2021

  • Telescope Version: 4.6.2
  • Laravel Version: 8.58.0
  • PHP Version: 8.0.10
  • Database Driver & Version: PostgreSQL 13.4
  • Octane: 1.0.11
  • Sail: 1.10.1

Description:

The use of the can middleware will cause the GateWatcher::recordGateCheck to throw an exception because the funcion getCallerFromStackTrace will return null instead of an array with the file and line.
I'm running Swoole but I suspect that the same will happen to RoadRunner, since the begin of the stacktrace it's not a vendor file.
Here is another issue talking about the same problem: #1072
Here is a link with the thrown exception: https://pastebin.com/DKMHPs3E.
If ignore_packages is set to false in the cofig file the error doesn't happen.

Steps To Reproduce:

  1. Create a new Laravel application with Sail
  2. Seed a new user
  3. Install Telescope
  4. Install Octane (swoole)
  5. Create a simple gate that only returns true and recive the authenticated user
  6. Create a route that needs to be authenticated and uses the can middleware with the created gate
@driesvints
Copy link
Member

@Gab-Menezes we're a bit strapped these days so we'll need to delay looking into this for now. If you could come up with a PR yourself that'd be great.

@derekmd
Copy link
Contributor

derekmd commented Sep 16, 2021

I put together a propspective fix but it only part of it may make sense for the GateWatcher: https://github.com/laravel/telescope/compare/4.x...derekmd:stack-trace-captures-supports-octane?expand=1

This would trace the stack back to file vendor/laravel/octane/src/ApplicationGateway.php:36 for call Laravel\Octane\ApplicationGateway->handle(Request $request). However I think it makes more sense to trace the gate check to midway through the stack at vendor/laravel/framework/src/Illuminate/Auth/Middleware/Authorize.php:43 line $this->gate->authorize($ability, $this->getGateArguments($request, $models));

<tr v-if="slotProps.entry.content.file">
<td class="table-fit font-weight-bold">Location</td>
<td>
{{slotProps.entry.content.file}}:{{slotProps.entry.content.line}}
</td>
</tr>

The UI hides file location details if it isn't stored so may be best to just introduce the src/Watchers/GateWatcher.php change that stores database entry 'file' => null when getCallerFromStackTrace() doesn't return a frame. QueryWatcher@recordQuery() already skips storing any Telescope entry if the returned stack trace is null.

Edit: I've submitted a pull request for the simplest fix. Telescope will provide less feedback about what triggered the gate check, but at least it doesn't throw an exception.

@Gab-Menezes
Copy link
Author

@derekmd ty for the fix, I was really busy with work this last week, and ended forgeting about it. I'm gonna test this later, should we close this issue or leave it until we can find a better fix ?

@derekmd
Copy link
Contributor

derekmd commented Sep 16, 2021

It can probably be closed because I can't think of a non-hacky fix for this. The problem seems to be route middleware performing gate authorization checks so App\* namespace controllers or middleware are never reached. Maybe the issue also occurs for controller constructor-configured authorization?

e.g.,

Route::get('profile/{user}', function (\App\User $user) {
    return view('profiles.view', ['user' => $user]);
})->middleware('can:view,user');

Even without using Laravel Octane, visiting /profile/1 generates this Telescope screen. The gate location is traced to the Laravel app's public/index.php bootstrap entrypoint which isn't useful information. Laravel Octane-enabled apps won't find any app-level source.

public-eh

As mentioned in my pull request, maybe a hardcoded fallback could be introduced to indicate Illuminate\Auth\Middleware\Authorize@handle() as the source. It's in the middle of the stacktrace:

auth

But the code is really hacky: https://github.com/laravel/telescope/compare/4.x...derekmd:gate-stacktrace-auth-middleware?expand=1 The Telescope screen hiding that "Location" line seems a good enough solution. The line will still be displayed for controller $this->authorize() or Blade template @can() calls.

@Gab-Menezes
Copy link
Author

Yeah I understand, looking carefully it really seems a hacky solution. I'm gonna let this issue be open for few more days, only to have a chance to attract another solutions from other people. I'm also gonna try to think in a solution, if nothing pops out I'm gonna close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants