monicahq / monica

Personal CRM. Remember everything about your friends, family and business relationships.
https://beta.monicahq.com
GNU Affero General Public License v3.0
21.78k stars 2.18k forks source link

Redirects to HTTPS while APP_URL is set to a non HTTPS HTTP URL #176

Closed rreuvekamp closed 5 years ago

rreuvekamp commented 7 years ago

Redirects seem to redirect to HTTPS, while the page itself uses regular HTTP, and the APP_URL in the .env also uses regular HTTP.

For instance: When submitting the login form, the browser correctly sends me to "http://example.org/login". It seems /login, though, points to "https://example.org/dashboard".

I've tried to rule out it being an Nginx issue.

If Monica doesn't support regular HTTPS, could that be said in the documentation? .env.example has a non-HTTPS URL as APP_URL though, so it should be supported I guess.

Maybe these items of .env are useful: APP_ENV=production APP_DEBUG=true APP_URL=http://php.example.org # For instance.

Thanks :) .

djaiss commented 7 years ago

This is because of https://github.com/monicahq/monica/blob/bb297d0c06b36363d25419aa7c32e191178edc92/app/Http/routes.php#L3

if (App::environment('production')) {
    URL::forceScheme('https');
}

I'm not sure I want to remove this though.

ciarans commented 7 years ago

+1 for the above comment - https is the future. Google already using it as a ranking system

ocdtrekkie commented 7 years ago

The problem is, forcing HTTPS isn't really viable when implementing HTTPS correctly is expensive and occasionally challenging, particularly if you're not doing something conventional. For a personal/everyone-use system, leaving it broken for HTTP is a very bad choice.

ocdtrekkie commented 7 years ago

Also, @ciarans, who exactly cares about Google's ranking system for a private database? Ranking worse would be a feature. And we shouldn't let Google's monopolies dictate terms on the Internet anyways.

ciarans commented 7 years ago

I get your Google point, however using https makes your system secure.

There are plenty of free, extremely easy solutions for SSL - cloudflare and Let's Encrypt to name a few. I currently use both in projects.

The only problem I can see is if you wanted to run this locally on a private database you can just used a self signed certificate? Again these are quick and easy to generate.

djaiss commented 7 years ago

Worst case, we can still add a new ENV variable to indicate whether we want to enforce HTTPS or not.

ocdtrekkie commented 7 years ago

@djaiss That'd be adequate.

@ciarans Neither solution you mention works in my setup, which is fairly common.

BranTurner commented 7 years ago

@ocdtrekkie Can you not just set APP_ENV to something other than production? Then HTTPS won't be forced anyway.

ocdtrekkie commented 7 years ago

@BranTurner There's an important question: What else does the production variable set? Is not setting the variable to production going to affect the software in other unintended ways?

BranTurner commented 7 years ago

@ocdtrekkie As far as I'm aware Laravel doesn't make any major changes to the app based off this, it just acts as a useful tool to allow the dev to prevent actions from occurring locally (e.g. forcing HTTPS). It makes changes to the artisan commands, but only insofar that it begins to asks "This app is in a production environment. Are you sure you want to run this command?" - it doesn't modify the actual commands.

To be sure, a quick search shows that it is only used to force HTTPS.

image

Obviously this would change if Monica begins to utilise this feature more, but currently I don't see a problem! :)

djaiss commented 7 years ago

@ocdtrekkie currently, APP_ENV is used for very few cases:

rreuvekamp commented 7 years ago

Sorry for the late reply. I did not expect this thread to get so many replies :) . Thanks for all your replies. I see it's an intended feature. Could this be made clear in the documentation?

asbiin commented 5 years ago

This is not a bug really

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.