Fixed #9179: Add option to force TLS connection

[kajes]

Description

The UrlGenerator in Laravel does not seem to work properly when the app is behind a reverse proxy, resulting in the application trying to get assets through a normal http connection. This gets blocked in modern browsers when accessing the site through a TLS connection.

Though this doesn't solve the UrlGenerator misbehaviour, this fix will give the user an option to force Laravel to use TLS connection even behind a reverse proxy, which seem to solve the issue.

Fixes #9179

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• This change requires a documentation update

How Has This Been Tested?

This has been tested in a live environment running in a Kubernetes cluster behind a nginx ingress controller reverse proxy. App version tested is 5.1.3.

• App uses TLS connection to fetch assets only when both of these conditions are true:
  • The APP_URL variable starts with https://
  • The APP_FORCE_TLS variable is set to true
• App does not force TLS connection if either of the above conditions are not met.

Test Configuration:

• PHP version: 7.2.24-0ubuntu0.18.04.7
• MySQL version: 10.5.9
• Webserver version: Apache/2.4.29
• OS version: Not sure. Application is deployed using the t3n/snipeit helm chart

Checklist:

• I have read the Contributing documentation available here: https://snipe-it.readme.io/docs/contributing-overview
• I have formatted this PR according to the project guidelines: https://snipe-it.readme.io/docs/contributing-overview#pull-request-guidelines
• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
  Note: Not sure how to test this automatically since it's very specific.
• New and existing unit tests pass locally with my changes
  Note: The tests don't seem to work either on the master branch or the develop branch without my commit

[welcome]

💖 Thanks for this pull request! 💖

We use semantic commit messages to streamline the release process and easily generate changelogs between versions. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix if it doesn't have one already.

Examples of commit messages with semantic prefixes:

• Fixed #<issue number>: don't overwrite prevent_default if default wasn't prevented
• Added #<issue number>: add checkout functionality to assets
• Improved Asset Checkout: use new notification method for checkout

Things that will help get your PR across the finish line:

• Document any user-facing changes you've made.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[snipe]

this fix will give the user an option to force Laravel to use TLS connection even behind a reverse proxy, which seem to solve the issue

So... the trusted proxies package that was made by Chris Fidao should already handle this. It was good enough that Laravel took it into core, and it’s no longer an optional package.

It would seem odd that you and three other people in the universe have a reverse proxy issue. I’m willing to entertain the idea, but I’m just not sure I like adding extra providers where I don’t think the problem is on our side.

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉

[snipe]

Thanks for this. I never heard back from you, so I decided to just merge it, but I'm still not entirely convinced it's necessary given the trusted proxies package being taken into core. You can't be the only people on the planet who use Laravel behind a funky proxy or other unusual config.

[grind42]

I'm deploying Snipe-IT on a Nomad cluster with LinkerD as a loadbalancer.
Although I had the APP_URL pointing to HTTPS and TRUSTED_PROXIES set Snipe-IT always tried to get the assets through HTTP which neither the browser nor the loadbalancer would allow.
Somehow the installation check still complains that theres a URL mismatch but everything is coming from HTTPS and it looks good to me. This might still be worth looking into.
Except for the installation check Kajes fix solved my issues. Thanks!