Page MenuHomePhabricator

Properly handle suppressed users
Closed, ResolvedPublic

Description

Global users can be suppressed (via CentralAuth), to make them disappear from everywhere. Once a user is suppressed, only privileged users would be able to see that the user exists. The methods in CentralIdLookup take an $audience parameter to determine whether suppressed users can be returned. Right now we bypass all checks by using AUDIENCE_RAW (see CampaignsCentralUserLookup), but this is clearly wrong and we should account for it throughout the whole extension.

QA notes: see T312772#8070367

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptJul 11 2022, 1:54 PM

Change 812893 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Handle deleted users

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

Change 812893 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Handle deleted users

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

@vaughnwalters I don't have specific testing notes for this, because username lookups happen all over the place. The only general guideline I have is: create an account, use it to create a registration and to register for another event, then suppress the account (you will need CentralAuth for this, see docs). Then, using another account, verify that the deleted username does not appear in any special page or API endpoint response either as a participant, organizer, or anything else.

I should also note that this is a bit bugged because the counts (number of participants/organizers) include deleted users, but the lists do not. I'm not sure how to fix this. It could be just me, but I find CentralIdLookup pretty hard to work with in its current state, so I filed T312821.

At any rate, I believe that showing inaccurate information if there's a suppressed user is better than showing suppressed usernames.

The previous patch introduced an additional small bug (aside from mismatching counts): if you open the "more details" dialog on the event page and a participant or organizer is suppressed, the respective section will have a "view full list" link even if there are no more organizers/participants to show. This will be fixed in r816790, so moving back to review.

Daimona updated the task description. (Show Details)

Change 816790 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Introduce CentralUser to replace ICampaignsUser

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

@Daimona To clarify, the current behavior we are fixing is to no longer display suppressed users, correct?

@Daimona To clarify, the current behavior we are fixing is to no longer display suppressed users, correct?

Correct. And in doing so, we're introducing a small bug where the count (in "X participants") is not in sync with the actual number of elements in the list. That part is hard to fix right now, so there's T312862 for doing that later on (probably needs a core change or something).

Change 816790 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Introduce CentralUser to replace ICampaignsUser

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

@Daimona a few things, and not all of these may be pertinent because some of it I used Special:BlockUser to block and used suppression mode, and then tested again using Special:CentralAuth to suppress the user


Using Special:BlockUser to block and suppress user from lists

The following event is created by the user V-suppress-test, and then after event creation that user was suppressed by at admin with suppression rights :

https://en.wikipedia.beta.wmflabs.org/wiki/Event:Suppress-test

Screen Shot 2022-08-16 at 12.53.07 AM.png (604ร—1 px, 132 KB)

The Event still displays and it shows that it is organized by the user who is blocked and is in suppression mode. What should we do with events that are created by users who are now blocked or in suppressed mode? At this point the organizer can't edit or update the event (unless the organizer is unblocked/unsuppressed), but the event still exists with the organizer's username attached to it.

Screen Shot 2022-08-16 at 1.04.40 AM.png (810ร—1 px, 119 KB)


Also, before being put in suppressed mode with Special:BlockUser V-suppress-test registered for the following event:

https://en.wikipedia.beta.wmflabs.org/wiki/Event:3

And when blocked and put in suppressed mode,V-suppress-test is still shown as registered for the event.

Screen Shot 2022-08-16 at 1.03.32 AM.png (848ร—1 px, 154 KB)


Now testing using Special:CentralAuth to suppress the user V-suppress-test, and re-testing for the same event,
https://en.wikipedia.beta.wmflabs.org/wiki/Event:3 :

in the API for the endpoint to check participants for the event, https://en.wikipedia.beta.wmflabs.org/w/rest.php/campaignevents/v0/event_registration/10/participants, a 500 error is thrown:

Screen Shot 2022-08-16 at 1.37.06 AM.png (578ร—1 px, 435 KB)


And in the API for the endpoint to list the organizers of an event, the user V-suppress-test who created the event and then was then suppressed by Special:CentralAuth is still shown, but I believe this user should not display as they are globally suppressed:
https://en.wikipedia.beta.wmflabs.org/w/rest.php/campaignevents/v0/event_registration/115/organizers

Screen Shot 2022-08-16 at 2.03.29 AM.png (848ร—1 px, 453 KB)


In the UI for https://en.wikipedia.beta.wmflabs.org/wiki/Event:Suppress-test created by the suppressed user, the organizer's user name is correctly not shown when blocked by Special:CentralAuth.

Screen Shot 2022-08-16 at 1.39.14 AM.png (918ร—1 px, 125 KB)


In the UI, for an event a user has joined but was then suppressed by Special:CentralAuth, the user is not shown but the count is incorrect (as already noted by @Daimona)

Screen Shot 2022-08-16 at 1.41.33 AM.png (910ร—1 px, 143 KB)

Using Special:BlockUser to block and suppress user from lists

The following event is created by the user V-suppress-test, and then after event creation that user was suppressed by at admin with suppression rights :

The Event still displays and it shows that it is organized by the user who is blocked and is in suppression mode. What should we do with events that are created by users who are now blocked or in suppressed mode? At this point the organizer can't edit or update the event (unless the organizer is unblocked/unsuppressed), but the event still exists with the organizer's username attached to it.

Just to confirm, the user was suppressed via Special:BlockUser and not CentralAuth, right? I tested this locally and what I observed is that the username was showing when I first reloaded the event page, but disappeared when I reloaded again; perhaps it was due to some caching or something like that. That said, I'm not sure what should be displayed -- certainly not "Organized by:" and then nothing.

Also, before being put in suppressed mode with Special:BlockUser V-suppress-test registered for the following event:

https://en.wikipedia.beta.wmflabs.org/wiki/Event:3

And when blocked and put in suppressed mode,V-suppress-test is still shown as registered for the event.

I can't reproduce this locally when suppressing via Special:BlockUser.

in the API for the endpoint to check participants for the event, https://en.wikipedia.beta.wmflabs.org/w/rest.php/campaignevents/v0/event_registration/10/participants, a 500 error is thrown:

I'll fix this.

And in the API for the endpoint to list the organizers of an event, the user V-suppress-test who created the event and then was then suppressed by Special:CentralAuth is still shown, but I believe this user should not display as they are globally suppressed:

Good point; we're only showing the ID and not the name, so it's probably less of a concern; let me think whether and how we may hide it.

Change 823623 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Adjust visibility of suppressed users

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

Using Special:BlockUser to block and suppress user from lists

The following event is created by the user V-suppress-test, and then after event creation that user was suppressed by at admin with suppression rights :

The Event still displays and it shows that it is organized by the user who is blocked and is in suppression mode. What should we do with events that are created by users who are now blocked or in suppressed mode? At this point the organizer can't edit or update the event (unless the organizer is unblocked/unsuppressed), but the event still exists with the organizer's username attached to it.

Just to confirm, the user was suppressed via Special:BlockUser and not CentralAuth, right? I tested this locally and what I observed is that the username was showing when I first reloaded the event page, but disappeared when I reloaded again; perhaps it was due to some caching or something like that. That said, I'm not sure what should be displayed -- certainly not "Organized by:" and then nothing.

Also, before being put in suppressed mode with Special:BlockUser V-suppress-test registered for the following event:

https://en.wikipedia.beta.wmflabs.org/wiki/Event:3

And when blocked and put in suppressed mode,V-suppress-test is still shown as registered for the event.

I can't reproduce this locally when suppressing via Special:BlockUser.

Correct, for the above two example in this comment, the user was blocked and added to suppressed mode using Special:BlockUser. In the video below, in the left panel I am blocking the user with my admin account, and in the right panel after, I am logged in an incognito window with the blocked user and after refresh you can see the name of the user still displaying in the event they created and then you can also see the user still displaying as an participant in an event they registered to attend.

Screen Recording 2022-08-16 at 11.48.43 AM.mov-68F4B17D-D4BE-4A51-AA69-033E8CA8F71A.gif (495ร—720 px, 3 MB)

Event where blocked user is attending:
https://en.wikipedia.beta.wmflabs.org/wiki/Event:3

Event with registration enabled by blocked user:
https://en.wikipedia.beta.wmflabs.org/wiki/Event:Suppress-test

When I block/suppress with Special:CentralAuth is when it shows Organized by: and then no user name, but when blocking with Special:BlockUser, the user name still displays.

Yes, for this example, the user was blocked and added to suppressed mode using Special:BlockUser. In the video below, in the left panel I am blocking the user with my admin account, and in the right panel after, I am logged in an incognito window with the blocked user and after refresh you can see the name of the user still displaying in the event they created and then you can also see the user still displaying as an participant in an event they registered to attend.

Screen Recording 2022-08-16 at 11.48.43 AM.mov-68F4B17D-D4BE-4A51-AA69-033E8CA8F71A.gif (495ร—720 px, 3 MB)

One thing I noticed from the video is that the checkbox to suppress the user is not checked, so the user is not actually being suppressed. You can verify that by noticing that the user contributions are still visible to unprivileged users, example.

Ah yes you're right @Daimona I forgot to check it on that demo. Look at the video below though where I did correctly block and suppress using Special:BlockUser - results are the same. Also, I will just leave that user V-suppress-test blocked and suppressed by Special:BlockUser (I had unblocked the user before) so the example does now show the Username or IP removed:

Screen Shot 2022-08-16 at 9.08.46 PM.png (846ร—2 px, 690 KB)

Screen Recording 2022-08-16 at 8.58.59 PM.mov-0726AE9A-B2E7-4F14-9AD5-BA00F663016E.gif (400ร—720 px, 2 MB)

So as it stands, user is blocked/suppressed by Special:BlockUser, user is shown as blocked here but the blocked/suppressed user still is showing at event they registered for and event they organized.

Ah yes you're right @Daimona I forgot to check it on that demo. Look at the video below though where I did correctly block and suppress using Special:BlockUser - results are the same. Also, I will just leave that user V-suppress-test blocked and suppressed by Special:BlockUser (I had unblocked the user before) so the example does now show the Username or IP removed:

Screen Shot 2022-08-16 at 9.08.46 PM.png (846ร—2 px, 690 KB)

Screen Recording 2022-08-16 at 8.58.59 PM.mov-0726AE9A-B2E7-4F14-9AD5-BA00F663016E.gif (400ร—720 px, 2 MB)

So as it stands, user is blocked/suppressed by Special:BlockUser, user is shown as blocked here but the blocked/suppressed user still is showing at event they registered for and event they organized.

Ohhhhhhhh I know what's going on here. The local wiki that I was using for testing does not have CentralAuth installed, but beta does. Our code uses central IDs, which can either be provided by Central Auth or be local, but NOT both, and CentralAuth takes precedence. The test user was suppressed locally but not globally; in fact, you can still see data for the global account here. Since the CentralAuth lookup regards the user as visible, irrespective of its local state, the user shows up. I'm not even sure if this would be considered a bug... Even if we fix it (which could be relatively complex), the username would still show up when viewing the event info on other wikis. I'd tentatively say this is working as intended, even if it may look surprising.

Ah yes you're right @Daimona I forgot to check it on that demo. Look at the video below though where I did correctly block and suppress using Special:BlockUser - results are the same. Also, I will just leave that user V-suppress-test blocked and suppressed by Special:BlockUser (I had unblocked the user before) so the example does now show the Username or IP removed:

Screen Shot 2022-08-16 at 9.08.46 PM.png (846ร—2 px, 690 KB)

Screen Recording 2022-08-16 at 8.58.59 PM.mov-0726AE9A-B2E7-4F14-9AD5-BA00F663016E.gif (400ร—720 px, 2 MB)

So as it stands, user is blocked/suppressed by Special:BlockUser, user is shown as blocked here but the blocked/suppressed user still is showing at event they registered for and event they organized.

Ohhhhhhhh I know what's going on here. The local wiki that I was using for testing does not have CentralAuth installed, but beta does. Our code uses central IDs, which can either be provided by Central Auth or be local, but NOT both, and CentralAuth takes precedence. The test user was suppressed locally but not globally; in fact, you can still see data for the global account here. Since the CentralAuth lookup regards the user as visible, irrespective of its local state, the user shows up. I'm not even sure if this would be considered a bug... Even if we fix it (which could be relatively complex), the username would still show up when viewing the event info on other wikis. I'd tentatively say this is working as intended, even if it may look surprising.

Okay @Daimona, I see. In this case then, should we document somewhere or note that suppression mode through Special:BlockUser has no effect of actually suppressing the user or blocking the user from being seen in our extension? Even if this is working as expected, it is confusing. Maybe @ifried has some thoughts on this?

โ€ข vyuen changed the task status from Open to In Progress.Aug 24 2022, 5:42 PM
โ€ข vyuen moved this task from Backlog to Darkship on the Campaign-Registration board.

Change 823623 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Adjust visibility of suppressed users

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

in the API for the endpoint to check participants for the event, https://en.wikipedia.beta.wmflabs.org/w/rest.php/campaignevents/v0/event_registration/10/participants, a 500 error is thrown:

I'll fix this.

Screen Shot 2022-08-25 at 3.33.46 PM.png (1ร—2 px, 292 KB)

Fixed ๐Ÿ‘. 500 error is no longer thrown.


And in the API for the endpoint to list the organizers of an event, the user V-suppress-test who created the event and then was then suppressed by Special:CentralAuth is still shown, but I believe this user should not display as they are globally suppressed:

Good point; we're only showing the ID and not the name, so it's probably less of a concern; let me think whether and how we may hide it.

Screen Shot 2022-08-25 at 3.34.51 PM.png (1ร—2 px, 181 KB)

Fixed ๐Ÿ‘. Organizer suppressed by Special:CentralAuth is no longer displaying at https://en.wikipedia.beta.wmflabs.org/w/rest.php/campaignevents/v0/event_registration/115/organizers

@Daimona should we address the Organized by: and then no user displaying in this ticket, or should that be a separate ticket? There is already a ton going on in this ticket.

This occurs if the organizer is suppressed or deleted through Special:CentralAuth
https://en.wikipedia.beta.wmflabs.org/wiki/Event:Suppress-test

Screen Shot 2022-08-25 at 4.07.06 PM.png (1ร—2 px, 218 KB)

@Daimona should we address the Organized by: and then no user displaying in this ticket, or should that be a separate ticket? There is already a ton going on in this ticket.

I believe it should be a separate task, because we may need input from Gregory and Ilana.

@Daimona should we address the Organized by: and then no user displaying in this ticket, or should that be a separate ticket? There is already a ton going on in this ticket.

I believe it should be a separate task, because we may need input from Gregory and Ilana.

I agree. I created T316294 to deal with this.

Moving this to product sign off but still am not sure how / if we should address my comment at T312772#8162643 ? @Daimona or @ifried have thoughts on this?

I see suppressed users hidden, as per the screenshot examples below. For the issue described in T312772#8162643, we will discuss it and determine next steps separately in T316693. For these reasons, I'm marking this ticket as Done.

Screen Shot 2022-08-30 at 4.04.32 PM.png (816ร—1 px, 94 KB)

Screen Shot 2022-08-30 at 4.04.26 PM.png (718ร—1 px, 74 KB)