opcodesio / log-viewer

Fast and beautiful Log Viewer for Laravel
https://log-viewer.opcodes.io
MIT License
3.49k stars 250 forks source link

Download file not sending additional headers #298

Closed mariosvasiliou closed 10 months ago

mariosvasiliou commented 10 months ago

I have added an index.blade.php custom header for authorization. All the requests are working normally, for fetching, deleting, etc except the download file. It seems that download file does not send the authorization header or any header you have added in additional headers on the blade file.

arukompas commented 10 months ago

Hey @mariosvasiliou Does that mean you cannot download log files? Because the request to download the file does not contain the custom authorisation headers?

mariosvasiliou commented 10 months ago

Hello @arukompas and thank you for the package and the reply. Yes if I add a custom header for authorization I cannot download files because they do not contain authorization headers.

arukompas commented 10 months ago

hey @mariosvasiliou , fix is ready in the latest update, v3.1.6 :)

mariosvasiliou commented 10 months ago

Thank you very much @arukompas you are awesome :D

mariosvasiliou commented 10 months ago

Hello @arukompas I have noticed something strange and it looks like it is still not working even though is passing headers now. For some reason is not going through middleware for API routes as specified in config api_middleware. It's redirecting and loading a script from GitHub, take a look at my screenshot... image

arukompas commented 10 months ago

hey @mariosvasiliou , yes, in order to download large files using axios or fetch (which is required in order to pass extra headers to the request, I had to find a way to stream the response as a download.

For that, I used this lib - https://github.com/jimmywarting/StreamSaver.js

It's a dirty solution, sure, but I'm not aware of any other way to pass headers in the previous solutions, which was a simple <a href="{download_link}" download>...</a>. If you have some ideas, they're most welcome!

Are you using any custom middleware? Would you be able to share the code, or describe what it does?

mariosvasiliou commented 10 months ago

Hello, @arukompas. Thanks for your message. Yes, I saw the library, but shouldn't load the library from the local environment and not from Git Hub? Yes, I am using a custom middleware that is registered in api_middleware on the config file. All API calls are passed through that middleware except download API. The middleware just verifies with a secret encrypted password that is the same as the header that is passed. In that case is not going through middleware so it does not verify that the header value is correct and cannot download the file. Perhaps an alternative solution is to add a request parameter on a gate check... Something like this one

 Gate::define('downloadLogFile', function (?User $user, LogFile $file,?$request) {
         // return true if the user is allowed to download the specific log file.
 });

Another solution that I am thinking of now is perhaps to add the middleware to the route

Route::get('files/{fileIdentifier}/download', 'FilesController@download')->name('log-viewer.files.download')->middleware(config('log-viewer.api_middleware'));

or to your controller download method like this

        /**
     * Instantiate a new controller instance.
     *
     * @return void
     */
    public function __construct()
    {
        $this->middleware(config('log-viewer.api_middleware'))->only('download');
    }
arukompas commented 10 months ago

hey @mariosvasiliou , thanks for the explanation!

I actually ended up going a different way completely, but it saves us from having this library.

So, in v3.1.9, the streamsaver library was removed and instead Log Viewer now uses Temporary Signed URLs to authorise and download log files/folders :) Seems to work great here. Could you also test it out and let me know what you think?

Thanks!

mariosvasiliou commented 10 months ago

Hello @arukompas Thank you for your support and your reply. I forgot completely the temporary signed URLs. good catch 👍 Just upgraded now, and it looks like that is not working, unfortunately, is not passing the custom set headers 😢 Perhaps now because is more secure I can bypass middleware checks or that route only.

arukompas commented 10 months ago

@mariosvasiliou it should only pass headers to the first request, such as /files/laravel.log/download/request. That's where the middleware is checked and authentication is checked as well. Once that is all good, it returns a temporary download URL so the frontend can safely download it without any other plugins.

I really want to help and to figure out what's going on, I'm just curious now :)

Could you share:

Thanks!

mariosvasiliou commented 10 months ago

Hello @arukompas , sorry for the late reply, too busy week ;p Well here is what I have on index.php

// Add additional headers for LogViewer requests like so:
  window.LogViewer.headers['Authorization'] = '{{base64_encode("an encrypted text")}}';

On my configurations I have

  'api_middleware' => [
        'secret_token_logs',
        \Opcodes\LogViewer\Http\Middleware\EnsureFrontendRequestsAreStateful::class,
        \Opcodes\LogViewer\Http\Middleware\AuthorizeLogViewer::class,
    ],

My middleware code is like that

      public function handle(Request $request, Closure $next)
    {
        $secretToken = \config('custom.api.encryption_key');
        if (! $request->hasHeader('Authorization') || ! $request->hasHeader('authorization')) {
            throw new AuthorizationException;
        }
        $givenToken  = $request->headers->get('Authorization', $request->header('authorization'));
        if (\blank($givenToken)) {
            throw new AuthorizationException;
        }
        $decrypted = \decryptData(\base64_decode($givenToken), \config('app.key'));
        if ($decrypted !== $secretToken) {
            throw new AuthorizationException;
        }
        return $next($request);
    }

Also, i have custom methods in my Service Provider like below (if you remove this, it still not working)

   LogViewer::auth(function ($request) {
            $isLocal = \isLocalEnvironment();
            if ($isLocal) {
                return true;
            }
            $user           = $request->user();

            return $this->userIsAllowed($user, $request);
        });

        Gate::define('downloadLogFile', function (?User $user, LogFile $file) {
            return $this->userIsAllowed($user);
        });
        Gate::define('downloadLogFolder', function (?User $user, LogFolder $folder) {
            return $this->userIsAllowed($user);
        });
        Gate::define('deleteLogFile', function (?User $user, LogFile $folder) {
            return $this->userIsAllowed($user);
        });
        Gate::define('deleteLogFolder', function (?User $user, LogFolder $folder) {
            return $this->userIsAllowed($user);
        });

The request headers for API have Authorization except for download endpoints. We are using Nginx. Thank you

arukompas commented 10 months ago

Great, thank you @mariosvasiliou !

By the way, after updating to the latest version last time, did you also re-publish the frontend assets with php artisan log-viewer:publish ?

mariosvasiliou commented 8 months ago

Hello @arukompas and happy New Year. Sorry for the late reply. I finally found the issue and I am sending it to inform you. The problem was in return $this->userIsAllowed($user); function I was checking if auth()->check() but for API calls the user is not authenticated... I managed to make it work by changing the header I was passing to the new header name and changing below to the following in the service provider...

private function userIsAllowed(?User $user = null, ?Request $request = null): bool
    {
        //bypass for api requests, since we are validating the token via middleware
        if ($request?->hasHeader('X-SOFTLINE-TOKEN')){
            return true;
        }

       //bypass for download requests, since are signed
        if ($request?->routeIs('log-viewer.files.download') || $request?->routeIs('log-viewer.folders.download')){
            return true;
        }

.......etc.......

Also in my middleware for secret_token_logs that I had on config, I have added this to bypass the check for specific routes

//bypass for download requests, since are signed
        if ($request->routeIs('log-viewer.files.download') || $request->routeIs('log-viewer.folders.download')){
            return $next($request);
        }

With those changes now downloading finally works for files and folders :D Thanks a lot, man, keep up the good work.