Page MenuHomePhabricator

Security review for EUCopyrightCampaign extension and EUCopyrightCampaignSkin
Closed, ResolvedPublic

Event Timeline

Extension:SkinPerPage

I'm not sure why this is needed, given the wiki will only have one page, so all pages could use the skin. But in any case, the extension is safe. I'm not entirely happy with the lack of validation of the skin name (T203143), but that really doesn't matter as invalid names get turned into the default.

EUCopyrightSkin

[As of af3b5408e0 which is marked WIP] This doesn't really look done yet, but I left some comments on the gerrit commit. Please also get various jenkins checks running for this, including phan-taint-check. No major issues so far.

EUCopyrightCampaign

[caf2c35de9] Same as skin. It looks like it is still very much a work in progress. Some concerns with how messages are escaped in js.

Additionally, it would be nice if there was a fallback for non-js users.

I'm not making any comment on privacy aspects of data storage. I assume that has been reviewed by other people.

Other comments

  • The special page transclusion reducing max parser cache time to 1 minute. On one hand, the page is so trivial caching it probably slower than rendering it. On the other hand, this seems unintentional, so you might want to override SpecialPage::getCacheTTL()
  • There are some comments about accept-language header via ULS. Keep in mind this is disabled on Wikimedia Wikis (via $wgULSAnonCanChangeLanguage) and generally requires specialized varnish config to work. Have you checked with Traffic

that this language workflow is workable.

chasemp added a project: Security-Team.

I'm not sure why this is needed, given the wiki will only have one page, so all pages could use the skin.

This allows the main content of the wiki to be displayed to visitors in the alternate skin without any of the wiki window dressing (sidebar, header, etc.):

but to still have access to those page elements from the default skin when editing or visiting supporting pages:

There are some comments about accept-language header via ULS. Keep in mind this is disabled on Wikimedia Wikis (via $wgULSAnonCanChangeLanguage) and generally requires specialized varnish config to work. Have you checked with Traffic that this language workflow is workable.

I have not but will do so. Thanks for mentioning this.

I'm not sure why this is needed, given the wiki will only have one page, so all pages could use the skin.

This allows the main content of the wiki to be displayed to visitors in the alternate skin without any of the wiki window dressing (sidebar, header, etc.):

but to still have access to those page elements from the default skin when editing or visiting supporting pages:
[..]

If this is for a single page only, then it might significantly simplify things to not build a skin for it, but instead merge the skin and the special page this wiki uses as its main page. Special pages have the ability to disable the skin handler, and do the output themselves instead. This is how the https://translatewiki.net/ homepage works, for example (Extension:TwnMainPage).

Using that method will presumably also simplify the backend code a bit, by not having to split the code between three places (skin, skin template, special page), rather just one (special page).

Setting the name of the special page directly at MediaWiki:mainpage then removes the need for transclusion, and with a few lines of config (example) it even be served with a clean server url, able to omit /wiki/.. for the main page. (As translatewiki.net does)

If this is for a single page only, then it might significantly simplify things to not build a skin for it, but instead merge the skin and the special page this wiki uses as its main page.

That sounds like a great alternate design. Since the site needs to go live next week and the security review is still in progress, it is unlikely that we'll have time to make that change at this point. However, that could be considered for a future enhancement.

Legoktm raised the priority of this task from Medium to High.Sep 1 2018, 2:50 AM

So some other things:

  • The actual dates and deadlines are confusing to me. Is the extension finished now? If not, when will it be "done". When is the target date for this all to go live?
  • What is the lifecycle plan for this (Will it be deployed forever? When will it be undeployed? Will the wiki be deleted after this is all done? )
  • The silverprop thing doesn't have any sort of captcha in, potentially making it vulnerable to automated sign ups (Not that captchas are really all that effective at stopping that). Is that acceptable? (I'm guessing yes, but wanted to mention it)
  • I just tried signing my email up - I got a success message, but no confirmation email. Does this mean anyone can sign anybody up to the policy mailing list without verifying they own this email? I find this concerning from a spam potential (If a malicious person signs thousands of randoms up for this email list, and people get mad over the spam, its Wikimedia which will take the blame. Not to mention people just accidentally putting in the wrong email address).
  • I just tried signing my email up - I got a success message, but no confirmation email. Does this mean anyone can sign anybody up to the policy mailing list without verifying they own this email? I find this concerning from a spam potential (If a malicious person signs thousands of randoms up for this email list, and people get mad over the spam, its Wikimedia which will take the blame. Not to mention people just putting in the wrong email address).

And the potential GDPR fines Wikimedia can face for unwanted spamming, given that you're targeting this to EU citizens in EU countries.

The actual dates and deadlines are confusing to me. Is the extension finished now? If not, when will it be "done". When is the target date for this all to go live?

We are still awaiting some final design tweaks from the designer (the look of the "phone your MEP" piece) that will minimally affect the layout. The text is also not final, but is getting close. A lot is happening simultaneously due to the tight timeline. The target date for going live is Wednesday.

What is the lifecycle plan for this (Will it be deployed forever? When will it be undeployed? Will the wiki be deleted after this is all done? )

It will be deleted at the end of the campaign.

I just tried signing my email up - I got a success message, but no confirmation email.

That is handled inside the separate mailing list management application, which is an established application used for other campaigns. I believe that functionality is not turned on yet, but I could be mistaken.

[offtopic]

The text is also not final, but is getting close.

Does it make sense to file "content bugs" for stuff on https://wmfeucc.demos.hallowelt.com/wiki/Fix_copyright! somewhere (the list of parliament members has a broken encoding and lists folks from different countries than selected)? If so, where?

[offtopic]

The text is also not final, but is getting close.

Does it make sense to file "content bugs" for stuff on https://wmfeucc.demos.hallowelt.com/wiki/Fix_copyright! somewhere (the list of parliament members has a broken encoding and lists folks from different countries than selected)? If so, where?

I have also noticed that and was wondering myself the same question.

Thanks for mentioning that! Maybe file those as subtasks of T203295 for now?

Thanks for mentioning that! Maybe file those as subtasks of T203295 for now?

Filed as T203323: EUCopyrightCampaign lists incorrect MEPs per country. Feel free to amend the task as appropriate. Thank you.

I just tried signing my email up - I got a success message, but no confirmation email.

That is handled inside the separate mailing list management application, which is an established application used for other campaigns. I believe that functionality is not turned on yet, but I could be mistaken.

That's right. This is handled in our email system and just not yet enabled.

Status: the code is currently frozen except for updating the i18n messages and adding one icon. I will let you know when those are complete. Could you please assess whether the items mentioned above have been satisfactorily resolved and whether any new issues have been introduced? Thank you!

Ill try to look at that before end of day.


Given this is 90% reviewed, im ok with this going on betawiki despite not fully reviewed yet (but full review must be done before production)

Great - thanks! All outstanding patches to the extension and skin are now merged. There will be one more patch tomorrow morning (German time) to each to add i18n files for German, French, and Spanish translations, but the rest of the code can be considered complete. We need to deploy to production tomorrow if at all possible. I really appreciate your help in getting this reviewed despite the extremely compressed time frame.

(Revised time estimate very first thing early tommorow morning)

@Bawolff Thanks for working on this. For the purpose of coordinating our announcement, what is the estimated time this will be done tomorrow morning?

@Bawolff Thanks for working on this. For the purpose of coordinating our announcement, what is the estimated time this will be done tomorrow morning?

I will be done prior to deployment window at 11 pdt

Ok. So I made 2 patches, provided that those are merged. This is approved subject to the following:

  • I assume that legal has approved the privacy aspects of tracking (please verify with legal):
    • via eventlogging: user language (in skin), selected issues, representative name, newsletter signup, and language (in ext)
    • via silverprop (if you agree to be on the mailing list): email, country, first name, last name
  • I assume that there doesn't need to be any separate privacy policy (for the tracked data) other than the normal main privacy policy (please confirm this with legal). In particular does it need a privacy statement like described in https://office.wikimedia.org/wiki/Survey_Privacy_Statement_Requirements_and_FAQ ?

Assuming the above is satisfied, I consider this approved

CCicalese_WMF updated the task description. (Show Details)
CCicalese_WMF updated the task description. (Show Details)

We should use the non-wiki privacy policy, linking the privacy policy page to: https://wikimediafoundation.org/privacy-policy/

And changing the email subscription text to:

[ ] I want to receive email updates about how to support Wikimedia. (You can unsubscribe at any time. This list is operated by [IBM Watson](https://www.ibm.com/privacy/details/us/en/privacy_shield.html).)

@CCicalese_WMF Can you help make these content changes?

With these changes, we're done with privacy review. Thanks again for the speedy work, @Bawolff

Change 458216 had a related patch set uploaded (by ItSpiderman; owner: ItSpiderman):
[mediawiki/extensions/EUCopyrightCampaign@master] Replace label text and privacy policy link

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

Change 458216 merged by jenkins-bot:
[mediawiki/extensions/EUCopyrightCampaign@master] Replace label text and privacy policy link

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

We should use the non-wiki privacy policy, linking the privacy policy page to: https://wikimediafoundation.org/privacy-policy/

Done (pointing to the wiki version instead for technical reasons): https://fixcopyright.wikimedia.org/wiki/MediaWiki:Privacypage

And changing the email subscription text to:

  • I want to receive email updates about how to support Wikimedia. (You can unsubscribe at any time. This list is operated by IBM Watson.)

@CCicalese_WMF Can you help make these content changes?

Done with https://gerrit.wikimedia.org/r/458216.

With these changes, we're done with privacy review. Thanks again for the speedy work, @Bawolff

Thanks, all! Marking as resolved.

Change 458256 had a related patch set uploaded (by Legoktm; owner: ItSpiderman):
[mediawiki/extensions/EUCopyrightCampaign@wmf/1.32.0-wmf.19] Replace label text and privacy policy link

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

Change 458260 had a related patch set uploaded (by Legoktm; owner: ItSpiderman):
[mediawiki/extensions/EUCopyrightCampaign@wmf/1.32.0-wmf.20] Replace label text and privacy policy link

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

Change 458260 merged by Legoktm:
[mediawiki/extensions/EUCopyrightCampaign@wmf/1.32.0-wmf.20] Replace label text and privacy policy link

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

Change 458256 merged by Legoktm:
[mediawiki/extensions/EUCopyrightCampaign@wmf/1.32.0-wmf.19] Replace label text and privacy policy link

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

Krinkle renamed this task from Security review of EU copyright stuff to Security review for EUCopyrightCampaign extension and EUCopyrightCampaignSkin.Jan 20 2019, 10:08 PM