[4.x] Don't log HTTP requests to Telescope endpoints

[derekmd]

Fixes #1126 introduced by #1124

Taylor rightly questioned why the domain check was an OR conditional as that ended up breaking Telescope's behavior using the out-of-the-box config:

return [
    'domain' => env('TELESCOPE_DOMAIN', null),
    'path' => env('TELESCOPE_PATH', 'telescope'), // e.g., https://example.com/telescope
    // ...
];

With Telescope's default config('telescope.domain') === null, the new method requestIsToApprovedDomain() was always returning true. This skips requestIsToApprovedUri() and permits every path to be logged. Telescope database entries are being inserted for /telescope/requests, /telescope/batches, etc. while also ignoring config('telescope.only_paths').

Instead make changes that allow the below config to be used, which the pull request originally intended to fix:

return [
    'domain' => 'https://telescope.example.com',
    'path' => null,
    // ...
];

1. requestIsToApprovedDomain() must check config('telescope.domain') contains a value for Telescope route registration. A request is approved for logging when Telescope's domain configuration is empty or its filled value is different from the current request meaning it's an app request (and not a Telescope admin request.)

   The issue with [4.x] Fix telescope.domain usage. #1124 was its fallback default value of config('telescope.domain', $request->getHost()) only gets used when 'domain' array key doesn't exist. config/telescope.php assigns that key the value null and fallback $request->getHost() was never returned.

2. requestIsToApprovedUri() will ignore config('telescope.path') === null for custom domains since conditional ! $request->is('*') would ignore all paths and nothing is ever logged.

3. When requestIsToApprovedDomain() returns true, config('telescope.only_paths') must also be checked. So that pull request should have introduced an AND conditional. The fixes in bullet points 1 & 2 allow requestIsToApprovedDomain() && requestIsToApprovedUri() to be used.

I wouldn't mind adding some integration and/or unit tests for this but any Laravel app code skipped by ! $app->runningInConsole() (like protected method handlingApprovedRequest()) is untestable with PHPUnit unless the code is restructured to make that mockable.

[frostfire64]

I wasted half a day working on this exact issue in a project I'm working on. I think this merge request should me merged ASAP because it can break the behavior of Telescope config for any user working with it that updates the package.

I can confirm that it broke the default and documented behavior of telescope. Which generated major confusion for me since it was a clean install of telescope done yesterday.

Downgrade to previous version did fix the issue.

[frostfire64]

I tested this on the newest release(4.6.4), the issue is solved. Thanks for swift action 💪

[EldonYoder]

Working great for me as well... thanks guys!