Page MenuHomePhabricator

CN: Stop using the geoiplookup HTTPS service (always use the Cookie)
Closed, ResolvedPublic2 Estimated Story Points

Description

The current CN GeoIP code tries to use the GeoIP cookie since https://gerrit.wikimedia.org/r/#/c/176727/ , but falls back to using geoiplookup.wikimedia.org when the Cookie lacks data (due to IPv6). We're shortly removing the IPv6 problem for the cookie (in a blocker for this task), and we'd like to get rid of the reference to the geoiplookup service in CN so we can shut it down ASAP.

Event Timeline

Change 305423 had a related patch set uploaded (by BBlack):
CN: use cookie exclusively for GeoIP data

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

Change 305423 abandoned by BBlack:
CN: use cookie exclusively for GeoIP data

Reason:
This approach is wrong for 3rd party - should keep most of the code and make it freegeoip.net -compatible.

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

@AndyRussG
There's a deadline for this, roughly Aug 23.

It sounds like the next steps are,

  • geoiplookup subdomain is set by configuration, defaulting to false which will prevent the call. Deploy WMF config where this is disabled.
  • We look at the Special:CentralNotice config for https://wikiapiary.com/w/index.php?title=Special:Ask&q=[[Has+extension%3A%3AExtension%3ACentralNotice&p=format%3Dtable%2Flink%3Dall%2Fheaders%3Dshow%2Fmainlabel%3D-2D%2Fintro%3D-3Cb-3EThis-20extension-20is-20in-20use-20on-20the-20following-20websites%3A-3C-2Fb-3E-3Cbr-20-2F-3E%2Fsearchlabel%3D%E2%80%A6-20further-20results%2Fdefault%3DThis-20extension-20is-20no-20longer-20in-20use-20on-20any-20website.%2Fclass%3Dsortable-20wikitable-20smwtable&po=%3FHas+website%3DWiki+name%0A%3FHas+MediaWiki+version%3DMediaWiki+version%0A%3FHas+extension+version%3DExtension+version%0A&sort=Has+website&order=rand&limit=500&eq=no | 3rd-party sites ]], and highlight the ones that have campaigns targeted by country. It's still possible that they're using the JS GeoIP variable of course.
  • Write to the 3rd parties to alert them that we're deprecating our public service.
  • On a slightly more relaxed schedule, update our code which consumes the GeoIP data and make it compatible with freegeoip.net variable names.

Adding Fundraising-Backlog so that this task makes it into our prioritization...

Change 306598 had a related patch set uploaded (by AndyRussG):
GeoIP: Make background lookup optional and configurable

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

Here's how things work in the proposed patch:

  • The mw.centralNotice.geoIP module itself no longer falls back to any background calls if the GeoIP cookie has no data or is invalid. It's all cookie, all the way. This is what we want for WMF production, I think.
  • A new config variable allows the activation of a separate RL module to look up location data via a third-party service. Such code is likely only of interest to non-WMF users of Mediawiki, and is only loaded when necessary. An example module to call and parse data from freegeoip.net is provided. Similar modules for other services should be simple enough to create.

This should ease the transition for other users of Mediawiki and CentralNotice.

P.S. Thanks to @Krinkle for the idea of using config to name a RL module, and to @BBlack and everyone else for your help, too!

Should we make a task for talking to third-party users? Announce the change on mediawiki-l?

Assuming code review goes fine and the patch is merged soon, I think the earliest we could deploy is Monday morning. Is that OK?

Monday morning's fine. IIRC from the meeting, the number of 3rd party wikis using CN (and we don't know that they also use GeoIP with it) is small. It's probably not worth an ML announcement on the balance of things, but we could reach out to them individually.

Monday morning's fine.

Great, thanks!!

IIRC from the meeting, the number of 3rd party wikis using CN (and we don't know that they also use GeoIP with it) is small. It's probably not worth an ML announcement on the balance of things, but we could reach out to them individually.

K... The other consideration I can think of that might make an announcement worthwhile is that some developers (including those working on stuff for WMF wikis) might be used to having geolocation work on their local mediawiki installations. Might not be a lot, 'cause they'd need to have CentralNotice installed... Still, for them, adding $wgCentralNoticeGeoIPBackgroundLookupModule = 'ext.centralNotice.freegeoipLookup' to LocalSettings.php might be useful.

On the other hand, a reason to hold off on an announcement might be that we'd want to announce the splitting off of GeoIP from CN pretty soon, so we might as well announce both changes at once. ;p

With a huge grain of salt. Wikiapiary isn't really maintained these days.

Heads up @Jhernandez, I think this implies there's a different, more canonical way to obtain the data via a promise in the places where we read out things like country code (e.g., https://meta.wikimedia.org/wiki/Schema:QuickSurveysResponses). CC @Mholloway and @Fjalapeno in case there are things in Android or iOS that use the cookies in either native land or the WebView/JS bridge.

If this affects window.Geo, could you coordinate to avoid breaking the community geonotice scripts? See https://en.wikipedia.org/wiki/Wikipedia:Geonotice#Technical_details (actual scripts are linked from there). Probably other wikis have the same or similar scripts.

This is used for geo-targeted notices on the watchlist page, which is a pretty important feature (used for organizing meetups and such).

The cookie itself isn't going away (and is normally set by our servers in the WMF case. The code that sets the cookie from JS is more to prevent unnecessary repeated calls to an HTTP service like freegeoip.net), and the fundamentals of window.Geo shouldn't be changing either, at least for the basic fields, I think?

If this affects window.Geo, could you coordinate to avoid breaking the community geonotice scripts?

Hi! Thanks for bringing this up...! Fortunately the latest patch-for-review and production changes leave both window.Geo and the cookie intact, so no worries...

Eventually, it might be nice to deprecate window.Geo. Sorry if the language in the commit message makes it seem like that's a definite thing... In truth it's just a proposal still. The new patch allows the same geo object to be fetched in a handler for the promise returned by mw.geoIP.getPromise(). Hopefully that'll be cleaner...

(BTW, if you know of code running on WMF production that's geolocating by calling the WMF's lookup service itself, pls see T100902.)

Change 306598 merged by jenkins-bot:
GeoIP: Background lookup optional and configurable, provide data via promise

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