I tested on 1.35, the REST API checks if the wiki is private, and denies access in that case.

user@bullseye:~$ curl -I "http://localhost/mediawiki/rest.php/localhost/v3/page/html/Main_Page/1?redirect=false&stash=true"
HTTP/1.1 403 Forbidden
Date: Tue, 11 Aug 2020 23:29:06 GMT
Server: Apache/2.4.43 (Debian)
X-Content-Type-Options: nosniff
X-Request-Id: 953e3769c76eb3d75366505d
Content-Type: application/json
user@bullseye:~$ curl "http://localhost/mediawiki/rest.php/localhost/v3/page/html/Main_Page/1?redirect=false&stash=true" | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    68    0    68    0     0   5230      0 --:--:-- --:--:-- --:--:--  5230
{
  "error": "rest-read-denied",
  "httpCode": 403,
  "httpReason": "Forbidden"
}

So seemingly no security problem. But, we're not forwarding cookies on private wikis, so zeroconf VE/Parsoid doesn't work.

I don't believe that Parsoid exports a 'save page' interface, so that access control is (still) being done by the Action API 'edit' action. So case 3 above shouldn't be an issue.

And I don't know that case 2 actually occurs (there was some user-specific permissions regarding deleted revisions, etc, but as I recall Parsoid solved this by removing that information from Parsoid output for all users).

So I *think* that we just need to double-check support for private wikis, where most pages are only viewable with a login.

@Legoktm Do you know where that permission check is being done? git grep 403 doesn't turn up anything in the Parsoid repo, so I assume this is being down by the REST infrastructure. I'd like to understand this better so I can be confident we don't accidentally break it...

Change 619593 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/VisualEditor@master] Support private wikis in Parsoid zero configuration mode

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

In T260201#6378213, @cscott wrote:

@Legoktm Do you know where that permission check is being done? git grep 403 doesn't turn up anything in the Parsoid repo, so I assume this is being down by the REST infrastructure. I'd like to understand this better so I can be confident we don't accidentally break it...

MWBasicRequestAuthorizer::isReadAllowed() does the underlying permission check.

Then BasicRequestAuthorizer::authorize() checks $this->handler->needsReadAccess(), which defaults to true (and Parsoid doesn't override).

Change 619593 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Support private wikis in Parsoid zero configuration mode

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

Change 619609 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/VisualEditor@REL1_35] Support private wikis in Parsoid zero configuration mode

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

Change 619609 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@REL1_35] Support private wikis in Parsoid zero configuration mode

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

I would prefer if someone (@cscott?) could verify my analysis in T260201#6378238 to double-check that we're OK from a security perspective before closing this.

Did a little more testing today.

It seems that we could use a speedy security review here to ensure that the Parsoid REST API doesn't:

1. For private wikis (login-required), doesn't expose page content w/o proper authentication

Verified in T260201#6378204, enforced by rest.php.

2. For wikis in general, doesn't expose content which should be subject to per-user restrictions (is that a thing?)

I revdeled a revision, and then tried to access it via rest.php, correctly received a 404:

$ curl "http://localhost/mediawiki/rest.php/localhost/v3/page/html/Test/2" | jq
{
  "message": "The specified revision is deleted or suppressed.",
  "httpCode": 404,
  "httpReason": "Not Found"
}

This is implemented in ParsoidHandler::respondToMissingRevisionContent.

3. For wikis in general, doesn't allow edits which should be subject to per-user restrictions

As cscott noted, Parsoid doesn't have any save API functionality. I did basic testing and VE correctly enforced proper permisions checks for page protection.

For wikis in general, doesn't expose content which should be subject to per-user restrictions (is that a thing?)

Yes, for instance if the Lockdown extension is used.
https://www.mediawiki.org/wiki/Extension:Lockdown

In T260201#6378204, @Legoktm wrote:

So seemingly no security problem. But, we're not forwarding cookies on private wikis, so zeroconf VE/Parsoid doesn't work.

I'm also quite sure this issue is currently not something RESTBase-related. Instead I'm quite sure we have found a VisualEditor Technical-Debt after an unhappy assumption in its current design.

(Just a moment to defend VisualEditor) anyway this design is somehow widespread, looking at this flow on the RESTbase documentation:

― File:Restbase request flow.svg

Look at the schema and notice the first blue arrow in the flow. I mean the MediaWiki → REST API blue arrow. That is a server-side HTTP call to reach itself.

This is the reasons I would be not surprised to see more and more comments about "strange server-side HTTP 403 Unauthorized errors when involving VisualEditor and a private wiki" filed here:

• T266971: "Error contacting the Parsoid/RESTBase server" (HTTP 404/403) and similar errors (catalog)

Some hints about how to fix this issue:

1. patch VisualEditor/includes/ApiParsoidTrait.php to pass user's cookies to its internal RESTBase HTTP request
2. refactor VisualEditor/includes/ApiParsoidTrait.php to be never called and avoid any kind of server-side HTTP request and relying instead of clean business logic in PHP
   1. eventually refactor RESTBase to provide a clean business logic in PHP (if actually only available via HTTP)

The first point should be enough because:

the API responds to the presence of a logged-in user and returns content appropriate to that user's permissions.
― API:REST API § Permissions and authorization

I was arguing with @Ferdi2005 about this behavior but we had some difficulties in describing this issue to other friends. Our feedback was something like:

So we gave birth to this nice metaphor/meme:

Alice with VisualEditor joins your new bar and asks for her sandwich. You (Bob) have that sandwich in the fridge. But serving sandwiches it's a job for your new waiter Carlos. Now the fun part. Carlos knows what to do. Carlos have to find that sandwich. Carlos knows a good bar making sandwiches. Carlos leaves the building. Carlos turns around. «Uh!» Carlos notices a cute bar. Carlos rings its intercom. At the same time, Bob welcomes a new costumer who looks exactly like Carlos (spoiler: he is Carlos). Carlos asks for a specific sandwich. Carlos looks at Bob. Bob looks at Carlos. Bob screams to Carlos: «Uhm! Who the hell are you? This is NOT your sandwich! This is reserved for Alice!» (HTTP 403). Carlos retraces his steps to find Bob and tell him this terrible news. That's why Carlos re-exits the bar, he finds again its original bar, he re-enters and comes back to Bob who was waiting with Alice. Carlos tells Bob that a weird guy blocked his request. Now. Alice is still waiting her sandwich. Bob reports to Alice this bad news: «Apologies Alice. As you can see, my staff reported you are not authorized to receive this sandwich. Nothing personal.»
Alice watches Bob. Bob watches Carlos. Alice looks at the fridge, so close. Alice cries.
Some seconds later, another costumer using wikitext enters your bar. Your ugly waitress Deborah goes to the fridge, she takes a sandwich and she throws that sandwich into the costumer's mouth in 13 milliseconds «Good morning, you want the usual I guess?» «Sure! Thanks Debor.. *gnam*». After this nice job Deborah goes back to do a lot of other useful things, for example polishing the floor for Bob with the right hand and cleaning the intercom for Carlos with the left one and serving other sandwiches to everyone with her cannon as usual.

O:)

In T260201#6678249, @valerio.bozzolan wrote:

Some hints about how to fix this issue:

1. patch VisualEditor/includes/ApiParsoidTrait.php to pass user's cookies to its internal RESTBase HTTP request

I already did this in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/619593 for private wikis. Is your problem that this isn't working for semi-private wikis that use Lockdown?

2. refactor VisualEditor/includes/ApiParsoidTrait.php to be never called and avoid any kind of server-side HTTP request and relying instead of clean business logic in PHP
   1. eventually refactor RESTBase to provide a clean business logic in PHP (if actually only available via HTTP)

This is the long-term plan.

In T260201#6690700, @Legoktm wrote:

I already did this in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/619593 for private wikis. Is your problem that this isn't working for semi-private wikis that use Lockdown?

Yup.

First of all let me say I think the check you introduced there has much sense.

But... I noticed that PermissionManager#isEveryoneAllowed( 'read' ) has an unexpected behavior for semi-private wikis. For these wikis it always returns true even if you cannot edit whatever page.

In short it seems it always returns true even if you have some protected namespaces with $wgNamespacePermissionLockdown:

<?php

...

// semi-private wiki with Lockdown (e.g. protected namespaces)

$is_public =
  MediaWikiServices::getInstance()
    ->getPermissionManager()
      ->isEveryoneAllowed( 'read' )


// expected false for semi-private wikis - but always true

Now.

I don't know all the consequences of altering the result of PermissionManager#isEveryoneAllowed() but I think that semi-private wikis may consider this workaround, while waiting to discover if this sort of thing should be fixed in the extension Lockdown or in the MediaWiki's core. This is a workaround:

<?php

...

// dirty fix for wikis with Lockdown + VisualEditor (REST)
// patch for VisualEditor/includes/ApiParsoidTrait.php
// https://phabricator.wikimedia.org/T260201
$wgHooks['OnUserIsEveryoneAllowed'][] = function( $right ) {
  return false;
};

This restores the expected behavior and will allow the propagation of user's cookies in the innermost layer of the REST API currently in use.