Page MenuHomePhabricator

Indef blocked users can get information exposed per mail
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. You have a wiki (like arbcom-de, where this happened), where the configuration is set, that blocked users can't login. On this wiki we block former members, so that they don't have access anymore
  2. You copy a text from the real wiki, like dewiki, where one of the former members get mentioned (the former member is indef blocked)
  3. The former member recives a mail about the notification, where Information like the page titel etc. gets exposed

Expected behaviour / proposed solution:
When blocking accounts disallows login, then you should not recive mail for notification when blocked.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 19 2018, 11:38 AM
Restricted Application added a project: Growth-Team. · View Herald TranscriptJul 19 2018, 11:44 AM

We are experiencing this problem on our arbcomwiki for de, that's why we are requesting as german arbcom to provide a quick fix please.

Catrope added a subscriber: Growth-Team.
Jalexander moved this task from Backlog to Security/Privacy on the Trust-and-Safety board.
Jalexander added a subscriber: Jalexander.

Related: T54453: Accounts blocked on wikis with wgBlockDisablesLogin enabled should not continue to receive email notifications, which fixed exactly the same issue but only for enotif emails, not Echo emails.

Catrope added a subscriber: Reedy.Aug 22 2018, 10:14 PM

Proposed patch:

@Reedy Mind if I have this ride along with T151910: Globally blocked users can still use Thanks for one consolidated security deploy after I get this reviewed?

Proposed patch:


@Reedy Mind if I have this ride along with T151910: Globally blocked users can still use Thanks for one consolidated security deploy after I get this reviewed?

Reviewing your patch:
One note:

+ ( $wgBlockDisablesLogin && $user->isBlocked() )

Can we add "|| $user->isBlockedGlobally()" to it? because isBlocked() returns none for globally blocked users and that's the whole reason we have T151910
Otherwise it looks good and I approve it

I don't think it would be correct to check isBlockedGlobally() in this case. Global blocks are IP-based, not user-based, and in the context where we're sending an email to a user we don't even have an IP to look at (or if we do, it's the IP of the user that caused the event, not the one that's receiving the notification).

SBisson assigned this task to Catrope.Aug 23 2018, 12:50 PM

@Catrope the patch looks good to me, and I agree with your point about isBlockedGlobally().

I don't think it would be correct to check isBlockedGlobally() in this case. Global blocks are IP-based, not user-based, and in the context where we're sending an email to a user we don't even have an IP to look at (or if we do, it's the IP of the user that caused the event, not the one that's receiving the notification).

I didn't know about this, thanks for clarification, +2 on my side.

Catrope changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 23 2018, 6:36 PM

Change 454881 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Catrope closed this task as Resolved.Aug 23 2018, 7:32 PM

Change 454912 had a related patch set uploaded (by Reedy; owner: Catrope):
[mediawiki/extensions/Echo@REL1_31] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454913 had a related patch set uploaded (by Reedy; owner: Catrope):
[mediawiki/extensions/Echo@REL1_30] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454914 had a related patch set uploaded (by Reedy; owner: Catrope):
[mediawiki/extensions/Echo@REL1_27] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454915 had a related patch set uploaded (by Reedy; owner: Catrope):
[mediawiki/extensions/Echo@REL1_29] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454912 merged by jenkins-bot:
[mediawiki/extensions/Echo@REL1_31] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454914 merged by jenkins-bot:
[mediawiki/extensions/Echo@REL1_27] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454913 merged by jenkins-bot:
[mediawiki/extensions/Echo@REL1_30] Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

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

Change 454915 abandoned by Reedy:
Don't send email notifs to blocked users if $wgBlockDisablesLogin is true

Reason:
No one cares about REL1_29 anymore

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