Bottelet / DaybydayCRM

DaybydayCRM an open-source CRM, to help you keep track of your daily workflow.
https://daybydaycrm.com
2.24k stars 753 forks source link

APP_URL being ignored causing Mixed Content errors #287

Closed jakubgs closed 2 years ago

jakubgs commented 2 years ago

I'm running 2.2.1 in a Docker Container with Nginx proxy, and another Nginx proxy on the host, and I have APP_URL set to my public https://crm.example.org URL, but I'm still getting "Mixed content" errors when loading the page:

image

But as far as I can tell the APP_URL is the correct setting: https://github.com/Bottelet/DaybydayCRM/blob/3c5ed12e217140859ff980161a742f8f58368151/config/app.php#L36-L42

According to Laravel docs for URL::asset() function the correct setting is ASSET_URL: https://laravel.com/docs/7.x/helpers#method-asset

But I tried that as well, and also APP_URI, but none of them work either. And I also have X-Forwarded-Proto and X-Forwarded-Host headers set on the proxy.

What's up with that?

jakubgs commented 2 years ago

I've managed to make it work by adding this line:

    'asset_url' => env('ASSET_URL', null),

https://github.com/laravel/laravel/blob/7.x/config/app.php#L57

Inside of the config/app.php file by hand. Not sure why it's missing from your configuration.

jakubgs commented 2 years ago

Despite fixing asset URL issues I can still see that POST requests - like when creating a project - are still done using HTTP:

app.js:1 Mixed Content: The page at 'https://crm.example.org/tasks/create' was loaded over HTTPS, but requested an insecure XMLHttpRequest endpoint 'http://crm.example.org/tasks'. This request has been blocked; the content must be served over HTTPS.

So APP_URL is clearly still being ignored.

jakubgs commented 2 years ago

Based on some suggestions I found I've made this change to the app:

diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php
index 4eb697f..a07d697 100644
--- a/app/Providers/AppServiceProvider.php
+++ b/app/Providers/AppServiceProvider.php
@@ -15,6 +15,7 @@ use App\Observers\ProjectObserver;
 use App\Observers\InvoiceObserver;
 use App\Repositories\Format\GetDateFormat;
 use Illuminate\Support\ServiceProvider;
+use Illuminate\Support\Facades\URL;
 use Laravel\Dusk\DuskServiceProvider;
 use Laravel\Cashier\Cashier;

@@ -27,6 +28,9 @@ class AppServiceProvider extends ServiceProvider
      */
     public function boot()
     {
+        if ($this->app->environment('production')) {
+            URL::forceScheme('https');
+        }
         Cashier::ignoreMigrations();
         Client::observe(ClientObserver::class);
         Task::observe(TaskObserver::class);

And that does resolve the issue with mixed content errors on API calls, but it's not exactly pretty.

You shouldn't have to change code to run an application in production.

Bottelet commented 2 years ago

@jakubgs I'm not sure why APP_URL behaves like this I'm sorry. What I've found so far is the solution you mentioned above as well.

jakubgs commented 2 years ago

Why did you close this? This isn't a solution, it's a hack fix. This needs a proper solution added to the codebase.

rm-yakovenko commented 2 years ago

Hi @jakubgs, @Bottelet!

May I throw in my 2 cents?

APP_URL config is only used when running in CLI mode. Otherwise, Laravel tries to guess host and schema from the current request.

https://github.com/laravel/framework/blob/f89fb45325483cb22ddfe51605199ea1725c0cec/src/Illuminate/Foundation/Bootstrap/SetRequestForConsole.php#L16-L34

And I also have X-Forwarded-Proto and X-Forwarded-Host headers set on the proxy.

IMHO, the correct way is to configure Laravel to trust the proxy server https://laravel.com/docs/8.x/requests#configuring-trusted-proxies

jakubgs commented 2 years ago

Right, but that's yet another thing that requires editing config files inside the docker file rather than just setting env vars.