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

Enhancement: use gethostbyname for dynamic trusted_proxies (like php of nextcloud does) #122

Open
jonathanmmm opened this issue Aug 14, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@jonathanmmm
Copy link

Hi, I have setup all correctly, but I get:

push server is not a trusted proxy, please add '172.22.1.1' to the list of trusted proxies or configure any existing reverse proxy to forward the 'x-forwarded-for' send by the push server.
As you can see, is 172.22.1.1 a dynamic ip from docker, I use multiple docker-compose files and and multiple networks (dynamic ips), is there any way to dynamically. Setting the ip of this docker network to static would make it less flexible, so something like gethostbyname would be great, if it would work also for notify_push:

nextcloud/documentation#7005 (comment)

@icewind1991
Copy link
Member

the push daemon doesn't check the trusted proxies itself, it just adds itself to the list of proxies the request has passed trough.
So any trick that can be used by nextcloud should work fine with notify_push to

@jonathanmmm
Copy link
Author

Here is the error in my docker container:

 Error:
nextcloud-notify_push    |    0: Failed to parse config
nextcloud-notify_push    |    1: Failed to parse nextcloud config
nextcloud-notify_push    |    2: Error while parsing php literal:
nextcloud-notify_push    |    2: .. |
nextcloud-notify_push    |    2: 79 |   'trusted_proxies' =>
nextcloud-notify_push    |    2: 80 |     array (
nextcloud-notify_push    |    2: 81 |       0 => gethostbyname('nginx'),
nextcloud-notify_push    |    2:    |            ^^ No valid token found, expected one of [Bool, Integer, Float, LiteralString, Null, Array, SquareOpen]
nextcloud-notify_push    |    2: 82 |     ),
nextcloud-notify_push    |    2: 83 | );
nextcloud-notify_push    |    2:

@ghost
Copy link

ghost commented Feb 28, 2022

I am facing the same issue. I need gethostbyname() in trusted_proxies for dynamic docker IPs which change on container recreation. Thats currently not possible with notify_push and results in the above error.

@HerrFrutti
Copy link

this would be great!

@joshtrichards joshtrichards added the enhancement New feature or request label Oct 6, 2023
@S0ulf3re
Copy link

I would like to see this as well. I am currently in the process of trying to follow this after upgrading to Nextcloud 29. And it feels like it might be a major piece to getting notify_push working within docker

@hampoelz
Copy link

I recently switched my Docker stacks from static IPs to hostnames, as this is much easier to maintain. Unfortunately, I also realized that push_notify no longer works in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants