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

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.

Quiddity updated the task description. (Show Details)
Quiddity raised the priority of this task from to Needs Triage.
Quiddity added a subscriber: Quiddity.
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: Security. · View Herald Transcript
Quiddity updated the task description. (Show Details)Oct 20 2015, 10:44 PM
Quiddity removed a project: 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: 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, 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".

I can't reproduce any issue here.

Mattflaschen-WMF closed this task as Invalid.EditedNov 13 2015, 6:18 PM
Mattflaschen-WMF claimed this task.

I tested like this:

  1. Create a board in the Flow_test_talk namespace locally.
  2. Create three topics as anon or non-privileged user
  3. Open the three topics at their permalink URL (both for easy URL-copying, and so if there were a caching issue it might trigger).
  4. Log in as Admin
  5. Delete the board.
  6. Check two of the topics as anon.
  7. Log in as a non-privileged user.
  8. 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.

Quiddity reopened this task as Open.Nov 13 2015, 6:46 PM

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

  1. used special:enableflow on https://www.mediawiki.org/wiki/User_talk:Quiddity_%28WMF%29/sandbox12
  2. created 2 test topics: https://www.mediawiki.org/wiki/Topic:Ssmt6e9sdonurd8w and https://www.mediawiki.org/wiki/Topic:Ssmt6ldw7dtvlbon
  3. deleted the page
  4. opened those test topics in various anon windows, including a different browser, and saw the topics still visible.
  5. 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

Mattflaschen-WMF renamed this task from Give admins deleting a board, an option to delete all topics at the same time. to Ensure users can not access topics from deleted boards via Varnish caching.Nov 13 2015, 10:11 PM

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:

As noted, the core patch is not required.

In T116095#1804896, @Mattflaschen wrote:

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

In T116095#1804896, @Mattflaschen wrote:

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?

csteipp added a subscriber: demon.Nov 17 2015, 1:16 AM

@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?

demon added a comment.Nov 17 2015, 3:02 AM

@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 :)

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?

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

csteipp closed this task as Resolved.Nov 18 2015, 9:08 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!

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

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

  1. 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 .

  1. '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
  2. 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.

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