Ensure users can not access topics from deleted boards via Varnish caching
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Quiddity
	Oct 20 2015, 10:40 PM

Description

When deleting a board, admins could be given a checkbox (off by default) to also delete all the individual topics attached to that board.

(or just the topics originating in that board? to account for cross-board workflows...)

Currently, at mediawiki, if just the board is deleted (not the topic), I can still see any topic that was on it, if I'm logged-out: e.g.
https://www.mediawiki.org/wiki/Topic:Sr5p6mvteztp9m8g
(which was created on https://www.mediawiki.org/wiki/User_talk:Quiddity_%28WMF%29/sandbox6 which I then deleted)

If a topic is deleted, I can only see the log entry if I'm logged-out, :
https://www.mediawiki.org/wiki/Topic:Sr5oy54aqpbmiyjs

There are ambiguous expectations, about whether all topics should be automagically deleted, when just the board is deleted.

Details

Subject	Repo	Branch	Lines +/-
Have Flow depend on Varnish	mediawiki/vagrant	master	+2 -2
Have SquidUpdate::newFromTitles use getSquidURLs	mediawiki/core	master	+1 -1
SECURITY: Evict topics from Squid/Varnish cache when board is deleted	mediawiki/extensions/Flow	master	+53 -0

Customize query in gerrit

Related Objects

Mentioned In: T119157: topic-title-wikitext conversion error when non-privileged user edits deleted topic
T119690: Flow topic pages are made inaccessible when board is deleted, but the topic pages are not deleted
T119156: Delete topics/board while another user tries to edit - a user gets confusing error messages
T116785: Try all possible admin actions concerning Flow
Mentioned Here: T119133: [betalabs] Flow\Exception\WikitextException on clicking deleted topic link from View history
T119156: Delete topics/board while another user tries to edit - a user gets confusing error messages
T90974: U3. Don't reuse board workflow when board is re-created
T98287: Merge topic lists when board is deleted, re-created, then undeleted.

Event Timeline

Quiddity created this task.Oct 20 2015, 10:40 PM

Quiddity raised the priority of this task from to Needs Triage.

Quiddity updated the task description. (Show Details)

Quiddity added a project: StructuredDiscussions.

Quiddity subscribed.

Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptOct 20 2015, 10:40 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

• Mattflaschen-WMF set Security to Software security bug.Oct 20 2015, 10:42 PM

Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptOct 20 2015, 10:42 PM

Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript

Restricted Application added a project: acl*security. · View Herald Transcript

Quiddity updated the task description. (Show Details)Oct 20 2015, 10:44 PM

Quiddity removed a project: acl*security.

Quiddity changed the visibility from "Custom Policy" to "Public (No Login Required)".

Quiddity changed the edit policy from "Custom Policy" to "All Users".

Quiddity changed Security from Software security bug to None.

Quiddity changed Security from None to Software security bug.

Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptOct 20 2015, 10:45 PM

Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript

Restricted Application added a project: acl*security. · View Herald Transcript

(Sorry, I was editing it, whilst you changed it)

Quiddity updated the task description. (Show Details)Oct 20 2015, 10:48 PM

Quiddity added a subscriber: Jdforrester-WMF.

@Quiddity, I get that the expectation is ambiguous, but if I understand it correctly, everything is in fact deletable, right? I.e., there's no way for someone to get content permanently up on the board in a way that can't be deleted?

If so, then we'll probably move this out of security and let you guys publicly figure out what is most desirable from a feature perspective.

@csteipp I'll ping @Mattflaschen for input as he marked it as security.

Yes, topics can be individually deleted, if done so prior to the board being deleted.
If the board is deleted first, then it becomes more difficult, in that admins have to view the deleted board e.g. here, and then open the permalink for each topic, in order to have access to the moderation (deletion) action.

@Mattflaschen , ping?

Currently, at mediawiki, if just the board is deleted (not the topic), I can still see any topic that was on it, if I'm logged-out: e.g.

https://www.mediawiki.org/wiki/Topic:Sr5p6mvteztp9m8g

I can't reproduce this (and it would be a serious bug).

I see:

Make sure you do a hard refresh.

IIRC, the topic title may be revealed some places, but that is analogous to title of a regular page being revealed.

I forget why I marked this as security (maybe just because I wasn't sure if it was reproducible?). We should maybe talk on IRC or Hangouts.

Also, I think there was going to be a separate bug about making that error more user-friendly.

Maybe there's a caching bug, if it previously showed up without that "Insufficient permissions".

• Mattflaschen-WMF mentioned this in T116785: Try all possible admin actions concerning Flow.Nov 6 2015, 7:48 PM

• Mattflaschen-WMF added a subscriber: Trizek-WMF.

I can't reproduce any issue here.

I tested like this:

Create a board in the Flow_test_talk namespace locally.
Create three topics as anon or non-privileged user
Open the three topics at their permalink URL (both for easy URL-copying, and so if there were a caching issue it might trigger).
Log in as Admin
Delete the board.
Check two of the topics as anon.
Log in as a non-privileged user.
Check the remaining topic.

For the non-privileged users/anons, I get "Insufficient permissions to execute this action."

If you can still reproduce this, please give steps.

Try the same steps at mediawiki.org - or for what I did specifically:

used special:enableflow on https://www.mediawiki.org/wiki/User_talk:Quiddity_%28WMF%29/sandbox12
created 2 test topics: https://www.mediawiki.org/wiki/Topic:Ssmt6e9sdonurd8w and https://www.mediawiki.org/wiki/Topic:Ssmt6ldw7dtvlbon
deleted the page
opened those test topics in various anon windows, including a different browser, and saw the topics still visible.
kept hard-refreshing, and I can still see them 10 minutes later.

I can reproduce it now at https://www.mediawiki.org/wiki/Topic:Ssmt6e9sdonurd8w (in a fresh browser, without even the logged out cookie).

I think we need to evict it from Varnish. I'll work on this today.

Varnish is actually testable locally now, but I didn't have this configured for my previous test, which is why I didn't see it locally.

I'm not going to block the Flow patch on the below, since it affects more code, but I think it is correct. Maybe we can put it up to Gerrit after the Flow patch is merged

0001-Have-SquidUpdate-newFromTitles-use-getSquidURLs.patch1007 BDownload

That also makes it more consistent with newSimplePurge.

I was able to reproduce (enable 'varnish' role and use http://127.0.0.1:6081/wiki/ for everything) and fix it locally:

0001-Evict-topics-from-Squid-Varnish-cache-when-board-is-.patch3 KBDownload

As noted, the core patch is not required.

• Mattflaschen-WMF edited projects, added Collaboration-Team-Archive-2015-2016; removed Collaboration-Team-Triage.Nov 13 2015, 11:18 PM

In T116095#1804896, @Mattflaschen wrote:

0001-Evict-topics-from-Squid-Varnish-cache-when-board-is-.patch3 KBDownload

Thanks! Can someone in collaboration review and find a time to deploy this next week?

In T116095#1804940, @csteipp wrote:

In T116095#1804896, @Mattflaschen wrote:

0001-Evict-topics-from-Squid-Varnish-cache-when-board-is-.patch3 KBDownload

Thanks! Can someone in collaboration review and find a time to deploy this next week?

Looks good to me. I'll deploy on Monday at 11am.

This is deployed now. @csteipp how should we proceed re making this public?

@Catrope, since Flow is still listed as beta on https://www.mediawiki.org/wiki/Extension:Flow, iirc, we've just merged the patch in gerrit once the cluster was patched, then announced it on wikitech-l.

@demon, does that sound right for Flow?

In T116095#1809844, @csteipp wrote:

@Catrope, since Flow is still listed as beta on https://www.mediawiki.org/wiki/Extension:Flow, iirc, we've just merged the patch in gerrit once the cluster was patched, then announced it on wikitech-l.

@demon, does that sound right for Flow?

Even if it was stable, we just announce it and move on, it's not a bundled extension :)

• Mattflaschen-WMF moved this task from Untriaged to In Development on the Collaboration-Team-Archive-2015-2016 board.Nov 17 2015, 5:18 PM

I've merged (Roan approved self-merging since it was already reviewed) and announced the Flow fix: https://gerrit.wikimedia.org/r/#/c/253760/

I put up the core fix as https://gerrit.wikimedia.org/r/253766 .

Are we ready to open this task?

• Mattflaschen-WMF moved this task from In Development to Needs Review on the Collaboration-Team-Archive-2015-2016 board.Nov 17 2015, 10:17 PM

Are we ready to open this task?

Yes please

• demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 18 2015, 6:31 AM

• demon changed the edit policy from "Custom Policy" to "All Users".

• demon changed Security from Software security bug to None.

Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptNov 18 2015, 6:31 AM

Change 253955 had a related patch set uploaded (by Mattflaschen):
Have Flow depend on Varnish

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

gerritbot added a project: Patch-For-Review.Nov 18 2015, 7:52 PM

Since the security issue is fixed, I'm going to close this now. Would be great to get the vagrant patch merged too.

If there's any other work I'm missing on this, feel free to reopen!

SBisson moved this task from Needs Review to QA Review on the Collaboration-Team-Archive-2015-2016 board.Nov 18 2015, 11:13 PM

Change 253955 merged by jenkins-bot:
Have Flow depend on Varnish

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

Etonkovidova mentioned this in T119156: Delete topics/board while another user tries to edit - a user gets confusing error messages.Nov 20 2015, 12:48 AM

Admin user that bookmarked (or just saved a permalink url) of a deleted topic or of a topic from the deleted board - still is able to see it.

Anyone else sees just 'Insufficient permission' warning .

'View history' displays deleted topic link that when clicked displays Error page - T119133: [betalabs] Flow\Exception\WikitextException on clicking deleted topic link from View history
There is some edge cases when topics/boards may be deleted while other users are still working on them - in such cases, confusing/tech jargon errors are displayed along with some weird display stuff. I filed it as T119156: Delete topics/board while another user tries to edit - a user gets confusing error messages

It'd be nice to detect immediately - upon any user's action on a Flow board - that a topic/board has been deleted and give a user straightforward feedback.

Etonkovidova moved this task from QA Review to Product Review on the Collaboration-Team-Archive-2015-2016 board.Nov 20 2015, 12:53 AM

• Mattflaschen-WMF mentioned this in T119690: Flow topic pages are made inaccessible when board is deleted, but the topic pages are not deleted.Nov 30 2015, 6:26 PM

Ricordisamoa subscribed.Jan 16 2016, 5:57 PM

Restricted Application added a subscriber: Luke081515. · View Herald TranscriptJan 16 2016, 5:57 PM

• Mattflaschen-WMF mentioned this in T119157: topic-title-wikitext conversion error when non-privileged user edits deleted topic.Apr 11 2016, 11:54 PM

• chasemp added a project: Security.Feb 10 2020, 10:53 PM

• chasemp removed a project: acl*security.Feb 20 2020, 8:14 PM

	F2961267: 0001-Evict-topics-from-Squid-Varnish-cache-when-board-is-.patch
	Nov 13 2015, 11:17 PM

	F2961083: 0001-Have-SquidUpdate-newFromTitles-use-getSquidURLs.patch
	Nov 13 2015, 10:01 PM

	F2912795: Topic restricted.png
	Nov 4 2015, 1:12 AM

Ensure users can not access topics from deleted boards via Varnish cachingClosed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

Ensure users can not access topics from deleted boards via Varnish caching
Closed, ResolvedPublic
Actions