Page MenuHomePhabricator

VE should always forward cookies when $wgVisualEditorParsoidForwardCookies is set
Closed, ResolvedPublic1 Story Points

Description

ApiVisualEditor.php ignores $wgVisualEditorParsoidForwardCookies if it thinks the wiki is publically readable.

@Catrope's initial commit didn't include this, but the check if the wiki was publicly readable was added by the time the patch was accepted. The rationale given ( Oct 23, 2013) was

Patch Set 2: Code-Review-1
Per discussion with Gabriel on IRC, this feature should refuse to work if there are no read restrictions. Will amend.

However, this ("refuse to work if there are no read restrictions") isn't documented anywhere and is counter-intuitive. The wiki I run is behind CA's SSO provider, for example, so it is unreadable without authentication no matter what the internal setting of MediaWiki is.

At the very least this should be better documented -- "If you set $wgVisualEditorParsoidForwardCookies make sure that you also set your wiki so that it is not publicly readable" -- but I think it would be better to honor the setting in all cases.

There are already a lot of disclaimers on the documentation for this, so we should assume that if someone sets it, they know what they're doing.

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 12 2016, 4:50 PM

Change 298526 had a related patch set uploaded (by MarkAHershberger):
Always forward cookies when $wgVisualEditorParsoidForwardCookies is set

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

Hmm. I'm minded to retain this security check. @GWicke, as this was your suggestion, what do you think?

Jdforrester-WMF triaged this task as Lowest priority.Jul 12 2016, 7:05 PM
Jdforrester-WMF set the point value for this task to 1.
Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.

Hmm. I'm minded to retain this security check. @GWicke, as this was your suggestion, what do you think?

It isn't really a security check. It is policy enforcement of possibly irrelevant policy. As I stated there are already other controls in place for this particular wiki that this "security check" is superfluous.

If you decide not to take this patch, then plenty of warnings should be added to the documentation for this.

The reason for not forwarding cookies to default-public wikis is that we do not want to cache or store specifically restricted content (ex: revision deleted pages) without also performing the requisite access checks. Doing so would circumvent content access restrictions configured on the wiki.

The wiki I run is behind CA's SSO provider, for example, so it is unreadable without authentication no matter what the internal setting of MediaWiki is.

So, is this a form of authentication that does not use MediaWiki's ACLs?

So, is this a form of authentication that does not use MediaWiki's ACLs?

All MediaWiki ACLs except 'read' can be used normally. The SSO component is only used for authentication and authorization of 'read' access. I've configured the wiki to create users automatically based on the information from the SSO component.

Before I discovered this undocumented feature, I had the wiki configured as a public wiki (since it was publicly readable before the introduction of SSO).

@GWicke: would you be willing to +2 the patch now?

ssastry moved this task from Backlog to Non-Parsoid Tasks on the Parsoid board.Jul 15 2016, 9:21 PM
Jdforrester-WMF closed this task as Resolved.Aug 15 2016, 11:10 PM

Change 298526 merged by jenkins-bot:
Always forward cookies when $wgVisualEditorParsoidForwardCookies is set

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