[go: up one dir, main page]

Page MenuHomePhabricator

Ensure country detected in Donate wiki
Closed, ResolvedPublic

Description

There are issues with GeoIP cookies to reaching this code that runs on Donate wiki for country detection:

https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/FundraiserLandingPage/+/refs/heads/master/includes/Specials/FundraiserRedirector.php

The very specific issue has been around for a while, but its impact has increased due to updates in how cross-site cookies are handled by browsers. (See T316578 for details.)

Event Timeline

Change 831243 had a related patch set uploaded (by AndyRussG; author: AndyRussG):

[mediawiki/extensions/FundraiserLandingPage@master] Try reloading once if no GeoIP cookie found

https://gerrit.wikimedia.org/r/831243

The attached patch should allow Donate Wiki to correctly detect the user's country in the following circumstance:

  • The user followed a direct link to Special:FundraiserRedirector.
  • A GeoIP cookie was not set in the user's browser for the wikimedia.org domain prior to this request.

In those circumstances, the PHP code that handles Special:FundraiserRedirector can't access the new GeoIP cookie that is sent to the user's browser on that request by Varnish.

The patch fixes the issue by reloading the page, adding a "reloaded=true" URL param, if no country was extracted from a GeoIP cookie in PHP. This works because on the reload, the GeoIP cookie that was set by Varnish on the first request is reflected back from the browser and is visible in PHP.

Rationale

I considered and rejected two alternate approaches to this problem:

  • Detect the cookie and set the country via JS. This would have still required a page reload, since between the first request to Special:FundraiserRedirector and the generation of the form based on the wiki template, there are no opportunities to run JavaScript.
  • In the PHP code for Special:FundraiserLandingPage (which Special:FundraiserRedirector redirects to) check if the country URL param is set to the default value, and if it is, try to replace it with a value from a GeoIP cookie, if available. This would have led to inconsistencies between the URL params and the form displayed. (Whenever the URL had a param country=XX, we would have tried to change the form's country based on the GeoIP cookie.) It seems preferable to avoid that inconsistency, since URLs can be copied and also are recorded in our server logs.

Testing locally

Steps to test this locally on the fundraising-dev docker setup:

<?php
wfLoadExtension( 'EventLogging' );
wfLoadExtension( 'FundraiserLandingPage' );
array_push( $wgWhitelistRead,
	'Special:FundraiserRedirector',
	'Special:LandingPage'
);
  • Log in to your local payments wiki (username is admin, pw is dockerpass).
  • Create the Template:Lp-layout-default template and save it with the same content as the template of the same name on Donate Wiki.
  • Create the Template:GeolocateError template and save it with the same content as the template of the same name on Donate Wiki.
  • Ensure that you have no GeoIP cookie set for localhost.
  • Visit Special:FundraiserRedirector. You should see some bits of wiki template code along with the text, "Sorry, we're having a little trouble detecting what country you're in."
  • Open a shell as root user in your payments container (docker-compose exec -u 0 payments bash).
  • In the container, edit /etc/apache2/sites-enabled/000-default.conf and add the line Header set Set-Cookie "GeoIP=US;path=/;Expires=Wed, Jan 01 2025 2:02:02 GMT" immediately before the VirtualHost section.
  • Restart the container's apache (apache2ctl graceful, or exit the shell and restart the container).
  • Visit Special:FundraiserRedirector. You should still see the text about the inability to detect the country.
  • Verify that a GeoIP cookie has been set for localhost.
  • Visit Special:FundraiserRedirector one more time. This time you shouldn't see the error text saying a country wasn't detected, but instead should see "country = US" (along with other stuff). This is the state production is currently in: the GeoIP cookie is set, and can be detected by PHP from the second HTTP request onward.
  • Delete the GeoIP cookie on localhost.
  • In src/payments/extension/FundraiserLandingPage, pull down the patch attached to this task.
  • Open the network tab in the developer tools of your browser.
  • Visit Special:FundraiserRedirector. Even though this is the first visit to the page with no cookie previously set, you shouldn't see the error text, but rather should see "country = US".
  • Review the network requests in your browser tools to see the expected sequence of redirects.

Also, here's a task to update docker to make it easier to work on this in the future: T317498.

Thanks!!!

Change 832663 had a related patch set uploaded (by AndyRussG; author: AndyRussG):

[mediawiki/extensions/FundraiserLandingPage@master] Only pass on params from URL query string

https://gerrit.wikimedia.org/r/832663

@Pcoombe there's a patch to detect the missing country and do the reload server-side, but it has the additional effect of discarding anything that was POSTed to the initial landing page, and only passing on those parameters that come in via the querystring. The current code will take any POSTed parameters along with the querystring parameters, and put them all on the querystring for the redirect.

Can you think of any scenario where a banner or some other form would be POSTing any parameters to the FundraiserLandingPage? I just want to make sure we're not breaking some obscure use case.

@Ejegg Thanks for checking. We used POST for some things years ago, but it's not used any more.

Change 832663 merged by jenkins-bot:

[mediawiki/extensions/FundraiserLandingPage@master] Only pass on params from URL query string

https://gerrit.wikimedia.org/r/832663

Change 831243 merged by jenkins-bot:

[mediawiki/extensions/FundraiserLandingPage@master] Try reloading once if no GeoIP cookie found

https://gerrit.wikimedia.org/r/831243

XenoRyet set Final Story Points to 2.