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

Fix for incorrect multisite dashboard Urls behind reverse proxy #18345

Merged
merged 1 commit into from Nov 19, 2021

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented Nov 18, 2021

Description:

Fixes #17945

When Matomo is running behind a reverse proxy server that forwards traffic to a directory path (eg. proxy.com/matomo/) but Matomo itself is installed in a different path (eg. someotherhost.com/ or someotherhost.com/differentpath) then incorrect paths are used for links in the Multi Sites page, specifically: site selection, the evolution sparklines and add a new website.

This PR is a simple fix to remove the calculated piwik_url from these links and use relative paths instead.

Tested with three docker images: mariadb:latest, matomo:4.5.0 official docker and nginx:latest

nginx configured with

location /matomo/ {
        proxy_set_header Host $host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Host $host;
        proxy_set_header X-Forwarded-Uri /matomo/;
        proxy_pass http://<matomo container ip>/;
        proxy_read_timeout 90;
    }

Matomo config.ini.php

[General]
assume_secure_protocol = 0
proxy_client_headers[] = "HTTP_X_FORWARDED_FOR"
proxy_host_headers[] = "HTTP_X_FORWARDED_HOST"
trusted_hosts[] = "<nginx container ip>"

Review

@bx80 bx80 added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 18, 2021
@bx80 bx80 added this to the 4.7.0 milestone Nov 18, 2021
@bx80 bx80 self-assigned this Nov 18, 2021
@sgiehl
Copy link
Member

sgiehl commented Nov 18, 2021

I'm not sure if removing the piwik_url from those urls is a good solution. Even though it might work, it would mean we need to adjust all twig templates that use the {{ piwikUrl }} variable for generating a url, as those urls would be incorrect as well. That means that likely the optout and other stuff might also not work.

@bx80 did you try if setting the config variable [General] proxy_uri_header = 1 solves the issue?

@bx80
Copy link
Contributor Author

bx80 commented Nov 18, 2021

@sgiehl I'd missed the proxy_uri_header option, yes that fixes the issue. This PR is therefore unnecessary. 🤦‍♂️

@bx80 bx80 removed this from the 4.7.0 milestone Nov 18, 2021
@bx80 bx80 added invalid For issues or pull requests that are no longer relevant to Matomo core. and removed not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 18, 2021
@bx80 bx80 closed this Nov 18, 2021
@tsteur
Copy link
Member

tsteur commented Nov 18, 2021

A lot of other links, even in twig templates, simply point to index.php. For example:

image

I think it be still an easy merge as it fixes it without having to configure something and AFAIK there's no downside to it? It doesn't mean we need to change all others now but the fix is there and it improves it quite a bit I would say.

@bx80 bx80 reopened this Nov 18, 2021
@bx80
Copy link
Contributor Author

bx80 commented Nov 18, 2021

Ok, I can merge these fixes and also mention the config setting in the original issue so there is an immediate solution for those affected.

@bx80 bx80 added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. and removed invalid For issues or pull requests that are no longer relevant to Matomo core. labels Nov 18, 2021
@bx80 bx80 added this to the 4.7.0 milestone Nov 18, 2021
@sgiehl sgiehl merged commit 45d8528 into 4.x-dev Nov 19, 2021
@sgiehl sgiehl deleted the m-17945-reverse-proxy-path-fix branch November 19, 2021 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultiSites reverse proxy on subdir/path is broken
3 participants