This was completed as part of T260604

When logged in, should see the popup on this page:
https://thegoodplace.wmflabs.org/index.php?title=Test&action=history

Without giving an example (and exposing my IP address) the hostname sometimes contains the IP address which would seem to be contradictory to the goals of IP masking. Alternatively, we could provide the top-level domain (TLD) of the host which I imagine would be sufficient?

@Pchelolo Awesome! Thanks!

@Pchelolo related question, should nested objects be null or should their (scalar) properties be null?

@Pchelolo That is super helpful, thank you so much!

In T262963#6466170, @chasemp wrote:

I had Alternatively, we could create a new web service in production that our extension could make a request to (from PHP). lingering in my brain when I made the packaging comment. Which was confusing on my part as I was doing a bit of a drive-by here.

In T260821#6461554, @Gilles wrote:

I'm told that MaxMind has a new competitor, https://db-ip.com/ which makes their dataset available in MaxMind's mmdb format: https://db-ip.com/db/ which means that the related tooling and libraries work with it. It's probably worth looking into their T&C as well. The free version of that database is CC BY licensed: https://db-ip.com/db/ip-to-location That could be a great starting point.

What do we want to do with this ticket?

In T260821#6453709, @Gilles wrote:

Since making an API that can return information about any IP address is probably out of the question, given that we shut down a service that did just that 4 years ago due to abuse.

@kamholz & @EvanProdromou this should work now, can you try again?

Added to the deployment schedule: https://wikitech.wikimedia.org/wiki/Deployments#deploycal-item-20200909T1800

This might be unfeasible, but could we modify the main docker-compose.yml to set the user for everyone? In other words, set it to the environment variables provided, if they aren't provided, use the default?

Why would the user set the environment variables and not set a LocalSettings.php ?

In T261053#6444540, @Tgr wrote:

Fair, but it doesn't seem like a big cost. MediaWiki JS will presumably provide an abstraction just like it does for the action API tokens, and all other clients will use OAuth anyway as obtaining a session cookie is much more cumbersome and fragile for them.

In T245474#6444492, @Tgr wrote:

Why is there a CORS request happening, anyway? I'm probably misremembering the OAuth2 flow but if the user is sent to the authorization dialog on the wiki, and that submits to a special page and that redirects to the rest API, that should be a same-domain request.

In T261053#6444336, @Tgr wrote:

OAuth does not rely on the browser to prevent an attack - the attack is prevented by the attacker not having any way to obtain the OAuth token.

In T245474#6444257, @Tgr wrote:

Doesn't oauth2/access_token expect redirected form POSTs from the authorization dialog? That's not preflighted.

In T232176#6442887, @eprodromou wrote:

Apparently the Web team does their development on localhost or other domains, but make API calls to the production domains, so they are running into this problem.

In T245474#6444103, @dbarratt wrote:

I don't think a preflight is necessarily a bug, it can be a feature. We've been talking about forcing a preflight on POST requests (like all other writable methods) by limiting the Content-Type being used on the request (See T126257) which would remove the need for CSRF tokens. As far as I can tell, the REST API only has routes that accept a non-standard Content-Type on a POST anyways.

In T245474#6441506, @Tgr wrote:

POST requests do not need to be preflighted, as long as the POST body is in some standard web format like application/x-www-form-urlencoded or multipart/form-data.

Moving this forward, we can iterate if we'd like to improve it.

In T261178#6439649, @Prtksxna wrote:

In T261178#6434950, @dbarratt wrote:

@Prtksxna What do you think? Do you like it exposing it on "See More" or should it scroll?

I don't think I understand the different between exposing and scrolling. Is what you're proposing different from what @Tchanders had demo'd during stand-up?

In T245474#6438414, @eprodromou wrote:

In T245474#6437372, @eprodromou wrote:

Yeah, the OAuth 2.0 endpoints aren't authenticated.

This is not entirely true. They do include a client ID and a client secret in the payload, but not a Cookie header.

I think the only thing that needs to happen is enabling $wgAllowCrossOrigin (assuming the request isn't authenticated)

In T260603#6436865, @Tchanders wrote:

We should probably define what fields we need and in what format they should take.

Do you mean which information we should return about the IP?

In T235554#6435256, @Addshore wrote:

For Fed props just setting $wgDisableOutputCompression = true; seems to work around this issue for us

I was thinking the reason would already be visable and the "See More" would just scroll the user upward with something like this:

window.scroll({
  top: 300, // Whatever the height of the space above the drawer is.
  behavior: 'smooth' // Works in modern browsers, ignored in older ones (afaik)
});

basically it would just smooth scroll the drawer to the top (or as high as it goes).

In T260604#6429413, @Tchanders wrote:

@Niharika Although we decided on Special:Investigate in the meeting, that page is complicated by the fact that it displays IP addresses of logged-in users. Displaying information for these IP addresses is more complicated, since they are not directly associated with a revision or log ID (the user name is instead; the IPs are discarded after 90 days). It might be confusing if we offer this feature only for anon IPs on Special:Investigate. Could we start with a different page that only displays IPs on anonymous editors, but still only make the feature available to checkusers to begin with?

In T194880#6310942, @Krinkle wrote:

Please explain why it matters for MediaWiki.

@Pchelolo That makes sense to me. This task should be resolved when T232176 is resolved correct? Assuming we make it configurable as described in T232176#6427076?

@Prtksxna Could you add a mockup or a screenshot to this task? Thanks!

We should probably define what fields we need and in what format they should take.

I think this whole thing is trying to make an exception for a single client. I don't think this is a good idea. But I do understand if a wiki wanted to take the performance impact for all routes. I just don't see why a wiki would want to allow it on some routes, but not others. It implies that the routes they want to allow use are known, when they might not be. We'd have to add configuration to allow a wiki to specify which routes are allowed.

I had an idea in T261696#6427063 that we should use Option 1 by default, but add a config variable to enable Option 3. Effectively a wiki could opt-into allowing cookie auth for cross-origin requests. This would be enabled on Meta, but disabled on all other wikis. This should fix @Pchelolo's dilemma while also limiting the performance impact.

Another idea, would be to allow Cookie Auth for all requests, but configure it per wiki. Basically Meta would allow such requests (by configuration) but all other wikis would not. Which I guess kind of makes sense conceptually. Meta is the "central" wiki for lots of things and should allow access all the time, but not the other way around.

TLDR: It's been decided that by default REST API will set access-control-allow-credentials: false thus disallowing CORS requests with cookie authentication.

I wonder if we should convert the table to wikitext on the server (with Parsoid PHP in core) before adding the attributes? That seems like it might be easier than ensuring they are all removed? 🤔

I think this is because of the mousedown handler https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/620798/6/modules/ext.checkuser.investigate.tables.js#234

This should happen automatically now that the repo has been created:
https://translatewiki.net/wiki/Translating:MediaWiki#Extensions

@Tchanders I added some clarity on which option we would be perusing and removed the questions. How does that look?

In T232176#6424790, @Pchelolo wrote:

@dbarratt is that your understanding as well? Do you want to amend your patch to support the revised proposal, or do you want me to take over? Alternatively, we can get your patch in as pure Option 1, and I'll do the revised proposal as a followup.

@Tgr to clarify, if there is no Origin header on the request, we can assume that it's the same-origin (it might not be, but that's not different than what we are doing now). Effectively modern browsers benefit from the added privacy/security, older browsers do not.

In T40417#6423925, @Tgr wrote:

Currently everyone can edit Wikipedia; with the change, people with certain browsers (possibly including the majority of certain countries, e.g. old IE versions are still very popular in China I believe) could not. That does not sound like an acceptable tradeoff for an (AIUI) mostly theoretical security hardening.

In T40417#6423655, @Tgr wrote:

Not all browsers set Origin reliably on non-CORS requests.

In T261358#6423664, @Pchelolo wrote:

Oh, what I have in mind is

protected Handler::needsCrossDomainAuthenticated() { return false; }

with developers overriding this. All the actual policy should be handled by the REST framework. Perhaps we'd want to the override slightly more elaborate to be able to limit the hosts beyond what the default allsowlist limits, TBD. The point is that developers will not be allowed to assign random CORS headers.

In T40417#6423655, @Tgr wrote:

Not all browsers set Origin reliably on non-CORS requests.

Let's take another example, should I be able to use the Content Translation tool on English without getting an OAuth token to the wiki I'm translating too? Or should I blindly be able to edit the other wiki with the cookie I have?

In T261358#6423445, @Pchelolo wrote:

Thus, we get the best of two worlds - do not loose a ton of performance for the majority of requests, only for the select few when we need. I do not see anything preventing us from having different policies for different routes

As part of T232176 I realized that a simple way to fix this problem is by rejecting write (POST) requests from users who 1) do not have a matching Origin to the allowlist and 2) are not logged in.

Welp, given all that, I would either use OAuth (when you need to make authenticated requests, ask the user to authenticate to Meta) or see if T232176 can be resolved in a different direction. I don't have an opinion on cross-origin cookie requests, other than it should be limited to the existing allowlist. It does present some performance concerns (i.e. Vary: Origin) that I don't have a very good solution for.

And another idea! You could do something similar to MediaWiki-extensions-CentralAuth and access another database in the cluster directly (I'm assuming the wiki is in the production cluster).

Another thought I had... would it be possible to deploy your extension onto Meta and use it there instead of a new wiki? Alternatively, would it be possible to move the OAuth central wiki to this new wiki?

AH! I forgot that Access-Control-Allow-Credentials is a response header for simple requests (not just OPTIONS). This means we'd need to add Vary: Origin to all requests if we want to support Option 2 or Option 3. We could add a query parameter with the origin, which might save a little bit of cache variance, but I can't imagine it would be a ton.

The patch I've submitted should do the trick. But please review. :)