Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #15374: load TrustProxies middleware in Kernel.php #15379

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

setpill
Copy link
Contributor

@setpill setpill commented Aug 22, 2024

Description

Loads the (already implemented) TrustProxies middleware.

Fixes a whole sleuth of issues pertaining to reverse proxies. People complaining about it have been pointed towards correctly configuring their trusted proxies; however it turns out even if they did, Snipe-IT failed to load the middleware responsible for taking them into account.

No additional dependencies required.

Before:

image

After:

image

Fixes #15374

Might also fix (non-exhaustive list):

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Relevant details: run the snipe/snipe-it Docker image on a machine, exposing port 80 of the container as port 8080 of the host (127.0.0.1 only if you prefer). Slap a reverse proxy in front of it. Make sure you set all the relevant headers for proxy stuff. If you're using nginx; that would be:

		proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
		proxy_set_header X-Forwarded-Host $host;
		proxy_set_header X-Forwarded-Proto $scheme;

On the Snipe-IT config side, make sure you set APP_URL to whatever you'd use to reach your reverse proxy. And set APP_TRUSTED_PROXIES=172.17.0.1. This is the IP address of the docker host from the perspective of the docker container, on linux. On other platforms, set it to whatever is appropriate. If you're not sure, 0.0.0.0/0 or * should probably work, but I haven't tested it because I do not use such platforms.

  • Test A

Open the correct app url that connects you to the reverse proxy (that proxies your request to the snipe-it container). See a big red error, because snipe-it thinks its hostname is localhost:8080 even though you told it to expect something else.

  • Test B

Apply the patch, change nothing else. Live in a sea of blissful green checkmarks.

Test Configuration:

  • PHP version: whatever is in snipe/snipe-it
  • MySQL version: not relevant
  • Webserver version: whatever is in snipe/snipe-it
  • OS version: Ubuntu 22.04

Checklist:

@setpill setpill requested a review from snipe as a code owner August 22, 2024 21:50
Copy link

welcome bot commented Aug 22, 2024

💖 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.

Copy link

what-the-diff bot commented Aug 22, 2024

PR Summary

  • Addition of TrustProxies Middleware
    This update introduces a new component known as TrustProxies to our system. In simpler terms, TrustProxies works like a security guard who verifies the identity of incoming traffic, ensuring that our online platform remains safe. It has been incorporated into the heart of our system (Kernel.php), enhancing our current security capabilities.

@snipe snipe merged commit fc5eb37 into snipe:develop Aug 23, 2024
9 checks passed
Copy link

welcome bot commented Aug 23, 2024

Congrats on merging your first pull request! 🎉🎉🎉

@snipe
Copy link
Owner

snipe commented Aug 23, 2024

Thanks!

@snipe snipe changed the title fixed #15374: load TrustProxies middleware in Kernel.php Fixed #15374: load TrustProxies middleware in Kernel.php Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants