Skip to content

Add trusted proxy config to framework configuration#1012

Merged
2 commits merged intomainfrom
add_trusted_proxy_config_to_framework
Aug 10, 2024
Merged

Add trusted proxy config to framework configuration#1012
2 commits merged intomainfrom
add_trusted_proxy_config_to_framework

Conversation

@ghost
Copy link

@ghost ghost commented Aug 10, 2024

The framework was not being configured with TRUSTED_PROXIES, I've added this configuration and .env variable to properly get the users actual IP as passed in the x-forwarded-for header, this should also resolve any rate limiter issues we could see if running behind a reverse proxy as $request->getClientIp() will return the x-forwarded-for IP if configured correctly.
https://symfony.com/doc/current/deployment/proxies.html#solution-settrustedproxies

@ghost ghost added enhancement New feature or request backend Backend related issues and pull requests labels Aug 10, 2024
@ghost ghost added this to the v1.8.0 milestone Aug 10, 2024
@ghost ghost self-assigned this Aug 10, 2024
@ghost ghost requested a review from BentiGorlich August 10, 2024 14:10
Copy link
Member

@BentiGorlich BentiGorlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the docker config have a blank TRUSTED_PROXIES? Or do we need a default here? The www container is forwarding requests to the php container isn't it?

@ghost
Copy link
Author

ghost commented Aug 10, 2024

Should the docker config have a blank TRUSTED_PROXIES? Or do we need a default here? The www container is forwarding requests to the php container isn't it?

Good point, we just need to swap the lines commented for docker. The container ips should fall within the private ranges I included.

@ghost ghost merged commit fb97be2 into main Aug 10, 2024
@ghost ghost deleted the add_trusted_proxy_config_to_framework branch August 10, 2024 15:33
thepaperpilot pushed a commit to thepaperpilot/mbin that referenced this pull request Aug 16, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Backend related issues and pull requests enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant