Page MenuHomePhabricator

Security review for cross-wiki aspects of Echo notifications
Closed, ResolvedPublic

Description

We'll be adding cross-wiki/global Echo notifications. We'd like a security review of the specific mechanism we choose for this (not yet implemented).

We expect this to be next quarter.

Event Timeline

Mattflaschen-WMF raised the priority of this task from to Needs Triage.
Mattflaschen-WMF updated the task description. (Show Details)
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptSep 16 2015, 6:25 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Legoktm moved this task from Backlog to External on the Notifications board.Sep 16 2015, 11:13 PM
revi added a subscriber: revi.Sep 17 2015, 9:31 AM
Catrope triaged this task as Normal priority.Sep 23 2015, 11:34 PM
Catrope added a subscriber: Catrope.

@Catrope is going to ask Chris about this and then decide whether it needs to be a blocker for Enable Cross-wiki Notifications by Default, T130655

csteipp added a subscriber: csteipp.Apr 6 2016, 6:13 PM

@jmatazzoni, is there a target date for making cross-wiki default?

I'd like to get started on this review this week, but I don't see enough information on this phab ticket. Could someone update the description of this phab ticket with the information noted at https://www.mediawiki.org/wiki/Wikimedia_Security_Team/Security_reviews#Requesting_a_review?

I briefly mentioned this (or at least something highly related to this) to @csteipp this morning. The relevant patch is https://gerrit.wikimedia.org/r/#/c/284677/ . It changes the way cross-wiki notifications are retrieved.

Currently, the API response from the server contains a list of foreign wikis where the user has notifications, and their API URLs. The client then makes cross-domain AJAX requests to these other wikis to retrieve notifications from each wiki.

With this patch, the API response from the server will still contain a list of wikis, but the client will now send an API request to the local wiki with a parameter like ?wikis=frwiki|dewiki|commonswiki . The server then uses curl_multi to make API requests to those wikis in parallel, gathers the results, and sends them to the client.

The benefit of using server-side fan-out instead of client-side fan-out is that we avoid problems with cross-domain AJAX requests: we've had issues with ad blockers, Privacy Badger and IE's privacy features thinking these requests are evil trackers and blocking them. (It's not obvious to software that *.wiktionary.org domains are not third-party domains.)

@dpatrick, heads up, collaboration would like to merge gerrit 284677 before the branch cut next Tuesday. Ping me if you need help getting that finished!

@dpatrick, heads up, collaboration would like to merge gerrit 284677 before the branch cut next Tuesday. Ping me if you need help getting that finished!

Is this gonna happen? "Next Tuesday" is now tomorrow...

@dpatrick, heads up, collaboration would like to merge gerrit 284677 before the branch cut next Tuesday. Ping me if you need help getting that finished!

Is this gonna happen? "Next Tuesday" is now tomorrow...

Given how close to the cut we are now, let's aim to get this into the next cut instead, but with plenty of time on beta labs before that, so merging on Wednesday or Thursday. If this misses the next cut, we will have to push back a publicly announced release date.

@Catrope, really sorry this is late. I've looked through https://gerrit.wikimedia.org/r/#/c/284677 and it looks ok. I'm fine if your team pushes this today.

@Catrope, really sorry this is late. I've looked through https://gerrit.wikimedia.org/r/#/c/284677 and it looks ok. I'm fine if your team pushes this today.

Thanks. I won't merge it this close to the cut because I'm concerned it needs proxy settings for curls to FQDNs like de.wikipedia.org to work on the cluster, but we'll merge it right after the cut, test in labs, and either cherry-pick it later this week or let it ride the next train.

I'm sorry I dropped the ball on this guys. I got really behind on stuff last week.

Catrope closed this task as Resolved.May 4 2016, 6:01 PM
Catrope claimed this task.

We've just merged it, and it'll go in next week's train. Thanks for the security review!

sbassett moved this task from Backlog to Done on the Security-Team board.Jun 11 2019, 7:13 PM