Page MenuHomePhabricator

CVE-2023-45369: pagetriagelist API leaks suppressed usernames
Closed, ResolvedPublicSecurity

Description

Reproduction steps

  • Create three users, "Admin" a user with oversighter privileges, "Person1" a user with normal user privileges, "Person2" a user with administrator privileges
  • Create a page using the "Person1" account (say "New Page")
  • Using the "Admin" account, block "Person1" with the the "Hide username from edits and lists" option checked.
  • Use the "Person2" account to review New Page and click on the information icon on the PageTriage toolbar.

Expected behaviour

Person1's name should not appear in the interface

Behaviour seen

Person2 is able to see the following text in the toolbar: "This page was created on 16 August 2023 by Person1 (talk | contribs)"

Details

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Suggested fix after chatting with Soda:

  • change the code that writes the metadata to write blank / "(hidden)" / "(suppressed)" to the username if the username of that revision is suppressed. oversighters can use normal tools such as page history if they need that information.
  • to capture suppressions after the initial article creation, add onSuppress() style hooks that do the same as above

I would not recommend going in the direction of adding a usernameIsSuppressed metadata field to pagetriage_page_tags SQL to track this. That would add a lot of complexity.

I would not recommend going in the direction of trying to decide if the user is a suppressor and then showing them the data if they are. It adds a lot of complexity.

Novem_Linguae renamed this task from PageTriage shows oversighted usernames to pagetriagelist API leaks suppressed usernames.Aug 18 2023, 9:34 AM

Related: {T234584}

This is probably very rare, but don't stewards have a hide username option too? We may need a similar ticket for that.

MediaWiki-extensions-CentralAuth

centralauth-suppress

Related: {T234584}

This appears to be the same issue, so this task should likely be merged into that existing task. Unless there's some subtle difference between the two that isn't immediately obvious.

@sbassett Revision deletion and suppression are different. Looks like both functionalities are in core now, so the main difference (I think) is the permissions deleterevision versus suppressrevision. But you're right, the fix for each will probably be similar.

Up to you. With that said, think we should merge or keep separate?

I spent some time yesterday coming up with a patch for this specific case (using User->isHidden()) lmk if there is anything I missed (which is probable since I'm not super familiar with the codebase).

@sbassett Revision deletion and suppression are different. Looks like both functionalities are in core now, so the main difference (I think) is the permissions deleterevision versus suppressrevision. But you're right, the fix for each will probably be similar.

Up to you. With that said, think we should merge or keep separate?

This should be about the (hideuser) permission imo.

Actually it appears there were a few linting errors with the previous patch, this should be the updated one:

Untested, but CR-1 based on reading the code:

diff --git a/i18n/en.json b/i18n/en.json
index edfd73f8..0c0e44d3 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -34,9 +34,11 @@
 	"pagetriage-no-author": "No author information present",
 	"pagetriage-byline": "Created by $1 ($2$3$4)",
 	"pagetriage-byline-heading": "Created by {{GENDER:$1}}",
+	"pagetriage-byline-hidden-username": "Created by ",
 	"pagetriage-byline-new-editor": "Created by new editor $1 ($2$3$4)",
 	"pagetriage-byline-new-editor-heading": "Created by a {{GENDER:$1|new editor}}",
 	"pagetriage-articleinfo-byline": "This page was created on $1 by $2 ($3$4$5)",
+	"pagetriage-articleinfo-byline-hidden-username": "This page was created on $1 by ",
 	"pagetriage-articleinfo-byline-new-editor": "This page was created on $1 by new editor $2 ($3$4$5)",
 	"pagetriage-editcount": "$1 {{PLURAL:$1|edit|edits}} since $2",
 	"pagetriage-author-not-autoconfirmed": "New editor",

Username should be a parameter to both messages, as not all languages use the same order of words as English does.

diff --git a/i18n/qqq.json b/i18n/qqq.json
index f8902fec..f64e32d3 100644
--- a/includes/Api/ApiPageTriageList.php
+++ b/includes/Api/ApiPageTriageList.php
@@ -27,6 +28,8 @@ class ApiPageTriageList extends ApiBase {
 		$opts = $this->extractRequestParams();
 		$pages = null;
 
+		$userFactory = MediaWikiServices::getInstance()->getUserFactory();

Please inject.

diff --git a/modules/ext.pageTriage.list/components/ListItem.vue b/modules/ext.pageTriage.list/components/ListItem.vue
index 50e317e1..048f61ce 100644
--- a/modules/ext.pageTriage.list/components/ListItem.vue
+++ b/modules/ext.pageTriage.list/components/ListItem.vue
@@ -185,6 +193,7 @@ module.exports = {
          */
 		// Creator information tags
 		creatorUserId: { type: Number, required: true },
+		creatorHidden: { type: Boolean, required: true },

This is marked as required: true, but not set in ApiPageTriageList::createUserInfo().

I made the changes requested as well as fixed some other related issues with the patch that came up while I was testing it (interop with wikilove/tagging etc).

Untested, but CR-1 based on reading the code:

diff --git a/i18n/en.json b/i18n/en.json
index edfd73f8..0c0e44d3 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -34,9 +34,11 @@
 	"pagetriage-no-author": "No author information present",
 	"pagetriage-byline": "Created by $1 ($2$3$4)",
 	"pagetriage-byline-heading": "Created by {{GENDER:$1}}",
+	"pagetriage-byline-hidden-username": "Created by ",
 	"pagetriage-byline-new-editor": "Created by new editor $1 ($2$3$4)",
 	"pagetriage-byline-new-editor-heading": "Created by a {{GENDER:$1|new editor}}",
 	"pagetriage-articleinfo-byline": "This page was created on $1 by $2 ($3$4$5)",
+	"pagetriage-articleinfo-byline-hidden-username": "This page was created on $1 by ",
 	"pagetriage-articleinfo-byline-new-editor": "This page was created on $1 by new editor $2 ($3$4$5)",
 	"pagetriage-editcount": "$1 {{PLURAL:$1|edit|edits}} since $2",
 	"pagetriage-author-not-autoconfirmed": "New editor",

Username should be a parameter to both messages, as not all languages use the same order of words as English does.

Yeah good point, hadn't thought about that.

diff --git a/i18n/qqq.json b/i18n/qqq.json
index f8902fec..f64e32d3 100644
--- a/includes/Api/ApiPageTriageList.php
+++ b/includes/Api/ApiPageTriageList.php
@@ -27,6 +28,8 @@ class ApiPageTriageList extends ApiBase {
 		$opts = $this->extractRequestParams();
 		$pages = null;
 
+		$userFactory = MediaWikiServices::getInstance()->getUserFactory();

Please inject.

Done

diff --git a/modules/ext.pageTriage.list/components/ListItem.vue b/modules/ext.pageTriage.list/components/ListItem.vue
index 50e317e1..048f61ce 100644
--- a/modules/ext.pageTriage.list/components/ListItem.vue
+++ b/modules/ext.pageTriage.list/components/ListItem.vue
@@ -185,6 +193,7 @@ module.exports = {
          */
 		// Creator information tags
 		creatorUserId: { type: Number, required: true },
+		creatorHidden: { type: Boolean, required: true },

This is marked as required: true, but not set in ApiPageTriageList::createUserInfo().

Changed this to creator_hidden and made sure to define it in ApiPageTriageList::createUserInfo()

Edit: Liniting errors

sbassett added a project: SecTeam-Processed.

If the above changes look good, we can get this patch deployed during this Monday's (2023-08-28) security deployment window.

That patch isn't attached to this task so I can't see it.

That patch isn't attached to this task so I can't see it.

Attached patch, not sure what happened with phabricator there :(

Novem_Linguae changed the task status from Open to In Progress.Aug 27 2023, 5:41 PM
Novem_Linguae assigned this task to Soda.
Novem_Linguae moved this task from Priority security to Code Review on the PageTriage board.

@taavi if you could get a chance to review this patch that would be great

@jsn.sherman @Scardenasmolinar If one of you has time, this security patch is awaiting review.

I can't see the patch either when I click on the link or perform a Gerrit search of the patch.

I can't see the patch either when I click on the link or perform a Gerrit search of the patch.

You will need to download the patch from the link, we are explicitly not using Gerrit since it is a security bug

I verified that the new patch from T344359#9121047 applies cleanly to ext:PageTriage@master and @wmf/1.41.0-wmf.24. phpcs and phan seem to run fine on both branches with the patch applied, save for a few (likely unrelated) PhanRedefinedClassReference for \Psr\Log\LoggerInterface with Echo (as PageTriage wants Echo installed since all hooks are under a single Hooks key within extension.json). The logic seems sound within ApiPageTriageList.php and the vue/js files, but I haven't tested it locally.

@Samwalton9-WMF @Scardenasmolinar @Soda - In attempt to make this easier, I hope, here is the full text of the patch from T344359#9121047. We could definitely use a bit more CR from subject matter experts on this before we security-deploy it. Thanks.

01-T344359.patch
From 644c70e74afa0a2a4a40e9957c71cd5768df8f75 Mon Sep 17 00:00:00 2001
From: Sohom <sohomdatta1+git@gmail.com>
Date: Wed, 23 Aug 2023 22:18:50 +0530
Subject: [PATCH] Don't expose usernames if user is hidden

- Remove user_name and user info from 'pagetriagelist' API
- Add user_hidden field to 'pagetriagelist' API
- Modify pagetriage toolbar to handle cases where no username
is provided
- Modify Special:NewPagesFeed to gracefully handle cases where
the username is hidden
- Add a special case in the Special:NewPagesFeed vue version
for hidden usernames (where other information is availiable)

Bug: T344359
Change-Id: I5714f69a70909e3c3e7322e5f3be62d837e4d1cc
---
 extension.json                                | 14 ++++++-
 i18n/en.json                                  |  2 +
 i18n/qqq.json                                 |  2 +
 includes/Api/ApiPageTriageList.php            | 37 +++++++++++++++----
 .../components/ListContent.vue                |  1 +
 .../components/ListItem.vue                   | 14 ++++++-
 .../models/ext.pageTriage.article.js          |  7 +++-
 .../ext.pageTriage.listItem.css               |  5 +++
 .../ext.pageTriage.listItem.underscore        |  8 +++-
 .../ext.pageTriage.views.toolbar/ToolView.js  |  1 +
 .../articleInfo.js                            | 10 ++++-
 .../articleInfo.underscore                    |  8 +++-
 modules/ext.pageTriage.views.toolbar/mark.js  |  3 +-
 modules/ext.pageTriage.views.toolbar/tags.js  |  2 +-
 .../ext.pageTriage.views.toolbar/wikiLove.js  |  7 +++-
 15 files changed, 104 insertions(+), 17 deletions(-)

diff --git a/extension.json b/extension.json
index 477d637a..648c7bdf 100644
--- a/extension.json
+++ b/extension.json
@@ -15,7 +15,12 @@
 		"MediaWiki": ">= 1.41"
 	},
 	"APIModules": {
-		"pagetriagelist": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageList",
+		"pagetriagelist": {
+			"class": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageList",
+			"services": [
+				"UserFactory"
+			]
+		},
 		"pagetriagestats": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageStats",
 		"pagetriageaction": {
 			"class": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageAction",
@@ -240,8 +245,11 @@
 				"pagetriage-mark-as-unreviewed",
 				"pagetriage-info-title",
 				"pagetriage-byline",
+				"rev-deleted-user",
+				"pagetriage-byline-hidden-username",
 				"pagetriage-byline-new-editor",
 				"pagetriage-articleinfo-byline",
+				"pagetriage-articleinfo-byline-hidden-username",
 				"pagetriage-articleinfo-byline-new-editor",
 				"pipe-separator",
 				"pagetriage-edits",
@@ -391,6 +399,8 @@
 				"pagetriage-recreated",
 				"pagetriage-no-author",
 				"pagetriage-byline",
+				"pagetriage-byline-hidden-username",
+				"rev-deleted-user",
 				"pagetriage-byline-new-editor",
 				"pagetriage-editcount",
 				"pagetriage-author-not-autoconfirmed",
@@ -587,6 +597,7 @@
 				"pagetriage-no-author",
 				"pagetriage-no-pages",
 				"pagetriage-byline",
+				"pagetriage-byline-hidden-username",
 				"pagetriage-byline-heading",
 				"pagetriage-byline-new-editor",
 				"pagetriage-byline-new-editor-heading",
@@ -602,6 +613,7 @@
 				"pagetriage-unreviewed-article-count",
 				"pagetriage-reviewed-article-count-past-week",
 				"pagetriage-unreviewed-draft-count",
+				"rev-deleted-user",
 				"pagetriage-sort-by",
 				"pagetriage-newest",
 				"pagetriage-oldest",
diff --git a/i18n/en.json b/i18n/en.json
index edfd73f8..552f7e4d 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -34,9 +34,11 @@
 	"pagetriage-no-author": "No author information present",
 	"pagetriage-byline": "Created by $1 ($2$3$4)",
 	"pagetriage-byline-heading": "Created by {{GENDER:$1}}",
+	"pagetriage-byline-hidden-username": "Created by $1",
 	"pagetriage-byline-new-editor": "Created by new editor $1 ($2$3$4)",
 	"pagetriage-byline-new-editor-heading": "Created by a {{GENDER:$1|new editor}}",
 	"pagetriage-articleinfo-byline": "This page was created on $1 by $2 ($3$4$5)",
+	"pagetriage-articleinfo-byline-hidden-username": "This page was created on $1 by $2",
 	"pagetriage-articleinfo-byline-new-editor": "This page was created on $1 by new editor $2 ($3$4$5)",
 	"pagetriage-editcount": "$1 {{PLURAL:$1|edit|edits}} since $2",
 	"pagetriage-author-not-autoconfirmed": "New editor",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index f8902fec..f64e32d3 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -53,10 +53,12 @@
 	"pagetriage-recreated": "Label indicating a page was previously deleted.",
 	"pagetriage-no-author": "Error message for missing page author information",
 	"pagetriage-byline": "Text indicating the page author.\n*$1 is a link to the author's user page.\n*$2 is a link to the author's talk page.\n*$3 is a separator character.\n*$4 is a link to the author's contributions.",
+	"pagetriage-byline-hidden-username": "Text indicating that a article was created by a author whoes name has been removed. This message is immediately followed by rev-deleted-user.",
 	"pagetriage-byline-heading": "Text indicating the page author. Parameters:\n* $1 - the author's username, used for GENDER support.",
 	"pagetriage-byline-new-editor": "Text indicating the page author (for when the author is a new editor). $1 is a link to the author's user page, $2 is a link to the author's talk page, $3 is a separator character, $4 is a link to the author's contributions.",
 	"pagetriage-byline-new-editor-heading": "Text indicating the page author (for when the author is a new editor). Parameters:\n* $1 - the 'author's username, used for GENDER support.",
 	"pagetriage-articleinfo-byline": "Text indicating the page author. $1 is the article creation date, $2 is a link to the author's user page, $3 is a link to the author's talk page, $4 is a separator character. $5 is a link to the author's contributions.",
+	"pagetriage-articleinfo-byline-hidden-username": "Text indicating that a article was created by a author whoes name has been removed in the article info section of the pagetriage toolbar. This message is immediately followed by rev-deleted-user.",
 	"pagetriage-articleinfo-byline-new-editor": "Text indicating the page author (for when the author is a new editor). Parameters:\n* $1 is the article creation date.\n* $2 is a link to the author's user page.\n* $3 is a link to the author's talk page.\n* $4 is a separator character\n* $5 is a link to the author's contributions.",
 	"pagetriage-editcount": "Display of page author's editing experience. $1 is total edit count, $2 is author's join date",
 	"pagetriage-author-not-autoconfirmed": "String indicating that the author was not yet autoconfirmed when the page was last edited",
diff --git a/includes/Api/ApiPageTriageList.php b/includes/Api/ApiPageTriageList.php
index a1e25ada..8b744631 100644
--- a/includes/Api/ApiPageTriageList.php
+++ b/includes/Api/ApiPageTriageList.php
@@ -3,12 +3,14 @@
 namespace MediaWiki\Extension\PageTriage\Api;
 
 use ApiBase;
+use ApiMain;
 use ApiResult;
 use MediaWiki\Extension\PageTriage\ArticleMetadata;
 use MediaWiki\Extension\PageTriage\OresMetadata;
 use MediaWiki\Extension\PageTriage\PageTriageUtil;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\Title\Title;
+use MediaWiki\User\UserFactory;
 use ORES\Services\ORESServices;
 use SpecialPage;
 use Wikimedia\ParamValidator\ParamValidator;
@@ -22,6 +24,18 @@ use Wikimedia\ParamValidator\TypeDef\IntegerDef;
  */
 class ApiPageTriageList extends ApiBase {
 
+	/** @var UserFactory */
+	private UserFactory $userFactory;
+
+	/**
+	 * @param ApiMain $query
+	 * @param string $moduleName
+	 */
+	public function __construct( ApiMain $query, string $moduleName, UserFactory $userFactory ) {
+		$this->userFactory = $userFactory;
+		parent::__construct( $query, $moduleName );
+	}
+
 	public function execute() {
 		// Get the API parameters and store them
 		$opts = $this->extractRequestParams();
@@ -72,12 +86,20 @@ class ApiPageTriageList extends ApiBase {
 					$metaData[$page]['creation_date']
 				);
 
-				// Page creator
-				$metaData[$page] += $this->createUserInfo(
-					$metaData[$page]['user_name'],
-					$userPageStatus,
-					'creator'
-				);
+				if ( $metaData[$page]['user_name'] ) {
+					// Page creator
+					$user = $this->userFactory->newFromName( $metaData[$page]['user_name'] );
+					if ( $user && !$user->isHidden() ) {
+						$metaData[$page] += $this->createUserInfo(
+							$metaData[$page]['user_name'],
+							$userPageStatus,
+							'creator'
+						);
+					} else {
+						$metaData[$page]['user_name'] = null;
+						$metaData[$page]['creator_hidden'] = true;
+					}
+				}
 
 				// Page reviewer
 				if ( $metaData[$page]['reviewer'] ) {
@@ -103,7 +125,7 @@ class ApiPageTriageList extends ApiBase {
 				}
 
 				$metaData[$page][ApiResult::META_BC_BOOLS] = [
-					'creator_user_page_exist', 'creator_user_talk_page_exist',
+					'creator_hidden', 'creator_user_page_exist', 'creator_user_talk_page_exist',
 					'reviewer_user_page_exist', 'reviewer_user_talk_page_exist',
 				];
 
@@ -219,6 +241,7 @@ class ApiPageTriageList extends ApiBase {
 			$prefix . '_user_talk_page_exist' => isset( $userPageStatus[$userTalkPage->getPrefixedDBkey()] ),
 			$prefix . '_contribution_page' => $userContribsPage->getPrefixedText(),
 			$prefix . '_contribution_page_url' => $userContribsPage->getFullURL(),
+			$prefix . '_hidden' => false,
 		];
 	}
 
diff --git a/modules/ext.pageTriage.list/components/ListContent.vue b/modules/ext.pageTriage.list/components/ListContent.vue
index c20b7f13..fff2148a 100644
--- a/modules/ext.pageTriage.list/components/ListContent.vue
+++ b/modules/ext.pageTriage.list/components/ListContent.vue
@@ -58,6 +58,7 @@ const listItemPropFormatter = ( pageInfo ) => {
 	listItemProps.revCount = parseInt( pageInfo.rev_count );
 	listItemProps.creationDateUTC = pageInfo.creation_date_utc;
 	listItemProps.creatorName = pageInfo.user_name;
+	listItemProps.creatorHidden = pageInfo.creator_hidden;
 	listItemProps.creatorAutoConfirmed = pageInfo.user_autoconfirmed === '1';
 	listItemProps.creatorRegistrationUTC = pageInfo.user_creation_date;
 	listItemProps.creatorUserId = parseInt( pageInfo.user_id );
diff --git a/modules/ext.pageTriage.list/components/ListItem.vue b/modules/ext.pageTriage.list/components/ListItem.vue
index 50e317e1..278444ec 100644
--- a/modules/ext.pageTriage.list/components/ListItem.vue
+++ b/modules/ext.pageTriage.list/components/ListItem.vue
@@ -58,14 +58,21 @@
 			</div>
 			<div class="mwe-vue-pt-info-row">
 				<div>
-					<span v-if="creatorName">
+					<span v-if="creatorName || creatorHidden">
 						<creator-byline
+							v-if="creatorName"
 							:creator-name="creatorName"
+							:creator-hidden="creatorHidden"
 							:creator-user-id="creatorUserId"
 							:creator-auto-confirmed="creatorAutoConfirmed"
 							:creator-user-page-exists="creatorUserPageExists"
 							:creator-talk-page-exists="creatorTalkPageExists"
 						></creator-byline>
+						<template v-else>
+							<span class="mwe-pt-history-suppressed">
+								{{ $i18n( 'pagetriage-byline-hidden-username', $i18n( 'rev-deleted-user' ) ) }}
+							</span>
+						</template>
 						<span v-if="creatorUserId > 0">
 							{{ $i18n( 'pagetriage-dot-separator' ).text() }}
 							{{ $i18n( 'pagetriage-editcount', creatorEditCount, creatorRegistrationPretty ).text() }}
@@ -185,6 +192,7 @@ module.exports = {
          */
 		// Creator information tags
 		creatorUserId: { type: Number, required: true },
+		creatorHidden: { type: Boolean, required: true },
 		creatorName: { type: String, required: true },
 		creatorEditCount: { type: Number, required: true },
 		creatorRegistrationUTC: {
@@ -433,4 +441,8 @@ module.exports = {
 .ores-pt-issues {
 	height: 0.55em;
 }
+.mwe-pt-history-suppressed {
+	text-decoration-line: line-through;
+	text-decoration-style: double;
+}
 </style>
diff --git a/modules/ext.pageTriage.util/models/ext.pageTriage.article.js b/modules/ext.pageTriage.util/models/ext.pageTriage.article.js
index 4bb28dfc..048e7c57 100644
--- a/modules/ext.pageTriage.util/models/ext.pageTriage.article.js
+++ b/modules/ext.pageTriage.util/models/ext.pageTriage.article.js
@@ -106,7 +106,12 @@ const Article = Backbone.Model.extend( {
 					article.get( 'creator_user_page_exist' )
 				)
 			);
-			article.set( 'user_contribs_title', article.get( 'creator_contribution_page' ) );
+		} else if ( article.get( 'creator_hidden' ) ) {
+			article.set( 'author_byline_html', mw.msg( 'pagetriage-byline-hidden-username', mw.msg( 'rev-deleted-user' ) ) );
+			article.set(
+				'user_title_url',
+				mw.msg( 'rev-deleted-user' )
+			);
 		}
 
 		// Are there any PageTriage messages on the talk page?
diff --git a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css
index 4d8f89e5..5016727f 100644
--- a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css
+++ b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css
@@ -123,6 +123,11 @@
 	display: table-cell;
 }
 
+.mwe-pt-history-suppressed {
+	text-decoration-line: line-through;
+	text-decoration-style: double;
+}
+
 .mwe-pt-potential-issues {
 	display: table-cell;
 	text-align: right;
diff --git a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore
index 887c9e05..a8a5b924 100644
--- a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore
+++ b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore
@@ -105,7 +105,13 @@
 				<div class="mwe-pt-info-row">
 					<div class="mwe-pt-author">
 					<% if ( typeof( user_name ) !== "undefined" ) { %>
-						<%= author_byline_html %>
+						<% if ( !creator_hidden ) { %>
+							<%= author_byline_html %>
+						<% } else { %>
+							<span class="mwe-pt-history-suppressed">
+								<%= author_byline_html %>
+							</span>
+						<% } %>
 						<!-- user_id is undefined or '0' for IP users -->
 						<% if ( typeof ( user_id ) != 'undefined' && Number( user_id ) !== 0 ) { %>
 							&#xb7;
diff --git a/modules/ext.pageTriage.views.toolbar/ToolView.js b/modules/ext.pageTriage.views.toolbar/ToolView.js
index 74a96746..f2544fec 100644
--- a/modules/ext.pageTriage.views.toolbar/ToolView.js
+++ b/modules/ext.pageTriage.views.toolbar/ToolView.js
@@ -267,6 +267,7 @@ module.exports = Backbone.View.extend( {
 			pageid: mw.config.get( 'wgArticleId' ),
 			title: mw.config.get( 'wgPageName' ),
 			creator: this.model.get( 'user_name' ),
+			creatorHidden: this.model.get( 'creator_hidden' ),
 			reviewed: reviewed
 		}, data );
 	},
diff --git a/modules/ext.pageTriage.views.toolbar/articleInfo.js b/modules/ext.pageTriage.views.toolbar/articleInfo.js
index bc1c2c7a..a6c52484 100644
--- a/modules/ext.pageTriage.views.toolbar/articleInfo.js
+++ b/modules/ext.pageTriage.views.toolbar/articleInfo.js
@@ -109,6 +109,8 @@ module.exports = ToolView.extend( {
 			url.toString()
 		);
 
+		const offset = parseInt( mw.user.options.get( 'timecorrection' ).split( '|' )[ 1 ] );
+
 		// creator information
 		if ( this.model.get( 'user_name' ) ) {
 			// show new editor message only if the user is not anonymous and not autoconfirmed
@@ -119,7 +121,6 @@ module.exports = ToolView.extend( {
 				bylineMessage = 'pagetriage-articleinfo-byline';
 			}
 
-			const offset = parseInt( mw.user.options.get( 'timecorrection' ).split( '|' )[ 1 ] );
 			// put it all together in the byline
 			// The following messages are used here:
 			// * pagetriage-articleinfo-byline-new-editor
@@ -150,6 +151,13 @@ module.exports = ToolView.extend( {
 				)
 			).parse();
 			this.model.set( 'articleByline_html', articleByline );
+		} else if ( this.model.get( 'creator_hidden' ) ) {
+			this.model.set( 'articleByline_html', mw.msg( 'pagetriage-articleinfo-byline-hidden-username', moment.utc(
+				this.model.get( 'creation_date_utc' ),
+				'YYYYMMDDHHmmss'
+			).utcOffset( offset ).format(
+				mw.msg( 'pagetriage-info-timestamp-date-format' )
+			), mw.msg( 'rev-deleted-user' ) ) );
 		}
 
 		const stats = [
diff --git a/modules/ext.pageTriage.views.toolbar/articleInfo.underscore b/modules/ext.pageTriage.views.toolbar/articleInfo.underscore
index 9a7ce587..9760e289 100644
--- a/modules/ext.pageTriage.views.toolbar/articleInfo.underscore
+++ b/modules/ext.pageTriage.views.toolbar/articleInfo.underscore
@@ -23,7 +23,13 @@
 	<!-- author info -->
 	<span class="mwe-pt-author">
 		<% if( typeof( user_name ) != 'undefined' ) { %>
-			<%= articleByline_html %>
+			<% if ( !creator_hidden ) { %>
+				<%= articleByline_html %>
+			<% } else { %>
+				<span class="mwe-pt-history-suppressed">
+					<%= articleByline_html %>
+				</span>
+			<% } %>
 			<div>
 				<!-- if user is registered (user_id is 0 for IP users) -->
 				<% if( typeof user_id != 'undefined' && Number( user_id ) !== 0 ) { %>
diff --git a/modules/ext.pageTriage.views.toolbar/mark.js b/modules/ext.pageTriage.views.toolbar/mark.js
index 4d347924..ea221f07 100644
--- a/modules/ext.pageTriage.views.toolbar/mark.js
+++ b/modules/ext.pageTriage.views.toolbar/mark.js
@@ -221,6 +221,7 @@ module.exports = ToolView.extend( {
 			status = this.model.get( 'patrol_status' ) === '0' ? 'reviewed' : 'unreviewed',
 			hasPreviousReviewer = this.model.get( 'ptrp_last_reviewed_by' ) > 0,
 			articleCreator = this.model.get( 'user_name' ),
+			articleCreatorHidden = this.model.get( 'creator_hidden' ),
 			previousReviewer = hasPreviousReviewer ? this.model.get( 'reviewer' ) : '';
 		let noteTarget = articleCreator,
 			notePlaceholder = 'pagetriage-message-for-creator-default-note',
@@ -239,7 +240,7 @@ module.exports = ToolView.extend( {
 			notePlaceholder = 'pagetriage-message-for-creator-default-note';
 		}
 
-		if ( mw.config.get( 'wgUserName' ) === articleCreator ) {
+		if ( mw.config.get( 'wgUserName' ) === articleCreator || articleCreatorHidden ) {
 			numRecipients--;
 			noteTarget = previousReviewer;
 			noteRecipientRole = 'reviewer';
diff --git a/modules/ext.pageTriage.views.toolbar/tags.js b/modules/ext.pageTriage.views.toolbar/tags.js
index 45ebfc61..b1d9a896 100644
--- a/modules/ext.pageTriage.views.toolbar/tags.js
+++ b/modules/ext.pageTriage.views.toolbar/tags.js
@@ -308,7 +308,7 @@ module.exports = ToolView.extend( {
 		if ( this.selectedTagCount > 0 ) {
 			$( '#mwe-pt-tag-submit-button' ).button( 'enable' );
 			$( '#mwe-pt-checkbox-mark-reviewed-wrapper' ).show();
-			if ( mw.config.get( 'wgUserName' ) !== this.model.get( 'user_name' ) ) {
+			if ( mw.config.get( 'wgUserName' ) !== this.model.get( 'user_name' ) && !this.model.get( 'creator_hidden' ) ) {
 				$( '#mwe-pt-tag-note' ).show();
 			}
 		} else {
diff --git a/modules/ext.pageTriage.views.toolbar/wikiLove.js b/modules/ext.pageTriage.views.toolbar/wikiLove.js
index f493b069..22fe7d0a 100644
--- a/modules/ext.pageTriage.views.toolbar/wikiLove.js
+++ b/modules/ext.pageTriage.views.toolbar/wikiLove.js
@@ -26,11 +26,14 @@ module.exports = ToolView.extend( {
 	render: function () {
 		// get the article's creator
 		const creator = this.model.get( 'user_name' );
+		const creatorHidden = this.model.get( 'creator_hidden' );
 
 		// get the last 20 editors of the article
 		const contributorArray = [];
 		this.model.revisions.each( function ( revision ) {
-			contributorArray.push( revision.get( 'user' ) );
+			if ( typeof ( revision.get( 'userhidden' ) ) === 'undefined' ) {
+				contributorArray.push( revision.get( 'user' ) );
+			}
 		} );
 
 		// count how many times each editor edited the article
@@ -54,7 +57,7 @@ module.exports = ToolView.extend( {
 		// set the Learn More link URL
 		$( '#mwe-pt-wikilove .mwe-pt-flyout-help-link' ).attr( 'href', this.moduleConfig.helplink );
 
-		if ( mw.user.getName() !== creator ) {
+		if ( mw.user.getName() !== creator && !creatorHidden ) {
 			// add the creator info to the top of the list
 			$( '#mwe-pt-article-contributor-list' ).append(
 				'<input type="checkbox" class="mwe-pt-recipient-checkbox" value="' + _.escape( creator ) + '"/>' +
-- 
2.41.0

Apologies for the slow response; we'll get this reviewed tomorrow.

We have taken a look at this patch and we have some comments and questions:

  • Does this patch purposefully hide anonymous users? I have tested this in my local environment and created an article with an anonymous user and it appears hidden, even if the IP isn't blocked.

Furthermore, there are a couple of observations in the UI.

In the curation toolbar, the date is struck out, which I don't think it should be.

Screenshot 2023-09-12 at 12.50.38.png (94×379 px, 11 KB)

The Created by label on the NewPagesFeed is also struck out when a user name is hidden. Only the (username removed) part of the string should be struck out.

Screenshot 2023-09-12 at 12.51.06.png (30×228 px, 3 KB)

We don't have the "Hide username from edits and lists" option in our local block pages; so we can't reproduce the issue until we have that sorted.

I think the issue with ip users is the way the check is handled in ApiPageTriageList.php
Here's a proposed patch to your patch that I think still does what you want.

We don't have the "Hide username from edits and lists" option in our local block pages; so we can't reproduce the issue until we have that sorted.

Are you testing in MediaWiki-Docker? Or MediaWiki-Vagrant? Suppress/delete functionality should be within MediaWiki core by default. You'd probably just want to create a suppress group and add a test user to it or just add various rights to the sysop group within LocalSettings.php. Here's an example of enwiki's suppress group config via shell.php:

> print_r( $wgGroupPermissions['suppress'] );
Array
(
    [browsearchive] => 1
    [deletedhistory] => 1
    [deletedtext] => 1
    [abusefilter-view-private] => 1
    [deleterevision] => 1
    [deletelogentry] => 1
    [hideuser] => 1
    [suppressrevision] => 1
    [suppressionlog] => 1
    [abusefilter-hide-log] => 1
    [abusefilter-hidden-log] => 1
    [viewsuppressed] => 1
    [oathauth-enable] => 1
)

You'd likely really only need to add hideuser to get the various controls to which I think you're referring.

We have taken a look at this patch and we have some comments and questions:

  • Does this patch purposefully hide anonymous users? I have tested this in my local environment and created an article with an anonymous user and it appears hidden, even if the IP isn't blocked.

Yes, that is unintended, I have attached a fixed patch below

Furthermore, there are a couple of observations in the UI.

In the curation toolbar, the date is struck out, which I don't think it should be.

Screenshot 2023-09-12 at 12.50.38.png (94×379 px, 11 KB)

The Created by label on the NewPagesFeed is also struck out when a user name is hidden. Only the (username removed) part of the string should be struck out.

Screenshot 2023-09-12 at 12.51.06.png (30×228 px, 3 KB)

I did look into doing this, but that would require having the unescaped HTML be a part of the message, which isn't considered a best-practice (AFAIK, since it could lead to XSS attack vectors) or spliting up the message in such a way that it wasn't translatable (which @taavi mentioned).

That being said, if there is another way to implement this, it might be worth doing it :)

We don't have the "Hide username from edits and lists" option in our local block pages; so we can't reproduce the issue until we have that sorted.

You should be able to add the admin user to the supressor group to gain those rights (on both Vagrant and MediaWiki-Docker)

I think the issue with ip users is the way the check is handled in ApiPageTriageList.php
Here's a proposed patch to your patch that I think still does what you want.

Yeah that's a good catch, I hadn't thought of testing with IP addresses, fixed in the patch below:

T344359.patch
From e23634cac9f4313435dfdf6210e811f422b8ef07 Mon Sep 17 00:00:00 2001
From: Sohom <sohomdatta1+git@gmail.com>
Date: Wed, 23 Aug 2023 22:18:50 +0530
Subject: [PATCH] Don't expose usernames if user is hidden

- Remove user_name and user info from 'pagetriagelist' API
- Add user_hidden field to 'pagetriagelist' API
- Modify pagetriage toolbar to handle cases where no username
is provided
- Modify Special:NewPagesFeed to gracefully handle cases where
the username is hidden
- Add a special case in the Special:NewPagesFeed vue version
for hidden usernames (where other information is availiable)

Bug: T344359
Change-Id: I5714f69a70909e3c3e7322e5f3be62d837e4d1cc
---
 extension.json                                | 14 ++++++-
 i18n/en.json                                  |  2 +
 i18n/qqq.json                                 |  2 +
 includes/Api/ApiPageTriageList.php            | 37 +++++++++++++++----
 .../components/ListContent.vue                |  1 +
 .../components/ListItem.vue                   | 14 ++++++-
 .../models/ext.pageTriage.article.js          |  7 +++-
 .../ext.pageTriage.listItem.css               |  5 +++
 .../ext.pageTriage.listItem.underscore        |  8 +++-
 .../ext.pageTriage.views.toolbar/ToolView.js  |  1 +
 .../articleInfo.js                            | 10 ++++-
 .../articleInfo.underscore                    |  8 +++-
 modules/ext.pageTriage.views.toolbar/mark.js  |  3 +-
 modules/ext.pageTriage.views.toolbar/tags.js  |  2 +-
 .../ext.pageTriage.views.toolbar/wikiLove.js  |  7 +++-
 15 files changed, 104 insertions(+), 17 deletions(-)

diff --git a/extension.json b/extension.json
index 3895ce32..7a0dd9f0 100644
--- a/extension.json
+++ b/extension.json
@@ -15,7 +15,12 @@
 		"MediaWiki": ">= 1.41"
 	},
 	"APIModules": {
-		"pagetriagelist": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageList",
+		"pagetriagelist": {
+			"class": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageList",
+			"services": [
+				"UserFactory"
+			]
+		},
 		"pagetriagestats": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageStats",
 		"pagetriageaction": {
 			"class": "MediaWiki\\Extension\\PageTriage\\Api\\ApiPageTriageAction",
@@ -239,8 +244,11 @@
 				"pagetriage-mark-as-unreviewed",
 				"pagetriage-info-title",
 				"pagetriage-byline",
+				"rev-deleted-user",
+				"pagetriage-byline-hidden-username",
 				"pagetriage-byline-new-editor",
 				"pagetriage-articleinfo-byline",
+				"pagetriage-articleinfo-byline-hidden-username",
 				"pagetriage-articleinfo-byline-new-editor",
 				"pipe-separator",
 				"pagetriage-edits",
@@ -390,6 +398,8 @@
 				"pagetriage-recreated",
 				"pagetriage-no-author",
 				"pagetriage-byline",
+				"pagetriage-byline-hidden-username",
+				"rev-deleted-user",
 				"pagetriage-byline-new-editor",
 				"pagetriage-editcount",
 				"pagetriage-author-not-autoconfirmed",
@@ -587,6 +597,7 @@
 				"pagetriage-no-author",
 				"pagetriage-no-pages",
 				"pagetriage-byline",
+				"pagetriage-byline-hidden-username",
 				"pagetriage-byline-heading",
 				"pagetriage-byline-new-editor",
 				"pagetriage-byline-new-editor-heading",
@@ -602,6 +613,7 @@
 				"pagetriage-unreviewed-article-count",
 				"pagetriage-reviewed-article-count-past-week",
 				"pagetriage-unreviewed-draft-count",
+				"rev-deleted-user",
 				"pagetriage-sort-by",
 				"pagetriage-newest",
 				"pagetriage-oldest",
diff --git a/i18n/en.json b/i18n/en.json
index edfd73f8..552f7e4d 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -34,9 +34,11 @@
 	"pagetriage-no-author": "No author information present",
 	"pagetriage-byline": "Created by $1 ($2$3$4)",
 	"pagetriage-byline-heading": "Created by {{GENDER:$1}}",
+	"pagetriage-byline-hidden-username": "Created by $1",
 	"pagetriage-byline-new-editor": "Created by new editor $1 ($2$3$4)",
 	"pagetriage-byline-new-editor-heading": "Created by a {{GENDER:$1|new editor}}",
 	"pagetriage-articleinfo-byline": "This page was created on $1 by $2 ($3$4$5)",
+	"pagetriage-articleinfo-byline-hidden-username": "This page was created on $1 by $2",
 	"pagetriage-articleinfo-byline-new-editor": "This page was created on $1 by new editor $2 ($3$4$5)",
 	"pagetriage-editcount": "$1 {{PLURAL:$1|edit|edits}} since $2",
 	"pagetriage-author-not-autoconfirmed": "New editor",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 7e4b3415..66e0e54d 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -53,10 +53,12 @@
 	"pagetriage-recreated": "Label indicating a page was previously deleted.",
 	"pagetriage-no-author": "Error message for missing page author information",
 	"pagetriage-byline": "Text indicating the page author.\n*$1 is a link to the author's user page.\n*$2 is a link to the author's talk page.\n*$3 is a separator character.\n*$4 is a link to the author's contributions.",
+	"pagetriage-byline-hidden-username": "Text indicating that a article was created by a author whoes name has been removed. This message is immediately followed by rev-deleted-user.",
 	"pagetriage-byline-heading": "Text indicating the page author. Parameters:\n* $1 - the author's username, used for GENDER support.",
 	"pagetriage-byline-new-editor": "Text indicating the page author (for when the author is a new editor). $1 is a link to the author's user page, $2 is a link to the author's talk page, $3 is a separator character, $4 is a link to the author's contributions.",
 	"pagetriage-byline-new-editor-heading": "Text indicating the page author (for when the author is a new editor). Parameters:\n* $1 - the 'author's username, used for GENDER support.",
 	"pagetriage-articleinfo-byline": "Text indicating the page author. $1 is the article creation date, $2 is a link to the author's user page, $3 is a link to the author's talk page, $4 is a separator character. $5 is a link to the author's contributions.",
+	"pagetriage-articleinfo-byline-hidden-username": "Text indicating that a article was created by a author whoes name has been removed in the article info section of the pagetriage toolbar. This message is immediately followed by rev-deleted-user.",
 	"pagetriage-articleinfo-byline-new-editor": "Text indicating the page author (for when the author is a new editor). Parameters:\n* $1 is the article creation date.\n* $2 is a link to the author's user page.\n* $3 is a link to the author's talk page.\n* $4 is a separator character\n* $5 is a link to the author's contributions.",
 	"pagetriage-editcount": "Display of page author's editing experience. $1 is total edit count, $2 is author's join date",
 	"pagetriage-author-not-autoconfirmed": "String indicating that the author was not yet autoconfirmed when the page was last edited",
diff --git a/includes/Api/ApiPageTriageList.php b/includes/Api/ApiPageTriageList.php
index a1e25ada..f6382cd1 100644
--- a/includes/Api/ApiPageTriageList.php
+++ b/includes/Api/ApiPageTriageList.php
@@ -3,12 +3,14 @@
 namespace MediaWiki\Extension\PageTriage\Api;
 
 use ApiBase;
+use ApiMain;
 use ApiResult;
 use MediaWiki\Extension\PageTriage\ArticleMetadata;
 use MediaWiki\Extension\PageTriage\OresMetadata;
 use MediaWiki\Extension\PageTriage\PageTriageUtil;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\Title\Title;
+use MediaWiki\User\UserFactory;
 use ORES\Services\ORESServices;
 use SpecialPage;
 use Wikimedia\ParamValidator\ParamValidator;
@@ -22,6 +24,18 @@ use Wikimedia\ParamValidator\TypeDef\IntegerDef;
  */
 class ApiPageTriageList extends ApiBase {
 
+	/** @var UserFactory */
+	private UserFactory $userFactory;
+
+	/**
+	 * @param ApiMain $query
+	 * @param string $moduleName
+	 */
+	public function __construct( ApiMain $query, string $moduleName, UserFactory $userFactory ) {
+		$this->userFactory = $userFactory;
+		parent::__construct( $query, $moduleName );
+	}
+
 	public function execute() {
 		// Get the API parameters and store them
 		$opts = $this->extractRequestParams();
@@ -72,12 +86,20 @@ class ApiPageTriageList extends ApiBase {
 					$metaData[$page]['creation_date']
 				);
 
-				// Page creator
-				$metaData[$page] += $this->createUserInfo(
-					$metaData[$page]['user_name'],
-					$userPageStatus,
-					'creator'
-				);
+				if ( $metaData[$page]['user_name'] ) {
+					// Page creator
+					$user = $this->userFactory->newFromName( $metaData[$page]['user_name'] );
+					if ( $user && $user->isHidden() ) {
+						$metaData[$page]['user_name'] = null;
+						$metaData[$page]['creator_hidden'] = true;
+					} else {
+						$metaData[$page] += $this->createUserInfo(
+							$metaData[$page]['user_name'],
+							$userPageStatus,
+							'creator'
+						);
+					}
+				}
 
 				// Page reviewer
 				if ( $metaData[$page]['reviewer'] ) {
@@ -103,7 +125,7 @@ class ApiPageTriageList extends ApiBase {
 				}
 
 				$metaData[$page][ApiResult::META_BC_BOOLS] = [
-					'creator_user_page_exist', 'creator_user_talk_page_exist',
+					'creator_hidden', 'creator_user_page_exist', 'creator_user_talk_page_exist',
 					'reviewer_user_page_exist', 'reviewer_user_talk_page_exist',
 				];
 
@@ -219,6 +241,7 @@ class ApiPageTriageList extends ApiBase {
 			$prefix . '_user_talk_page_exist' => isset( $userPageStatus[$userTalkPage->getPrefixedDBkey()] ),
 			$prefix . '_contribution_page' => $userContribsPage->getPrefixedText(),
 			$prefix . '_contribution_page_url' => $userContribsPage->getFullURL(),
+			$prefix . '_hidden' => false,
 		];
 	}
 
diff --git a/modules/ext.pageTriage.list/components/ListContent.vue b/modules/ext.pageTriage.list/components/ListContent.vue
index 7385fff3..d79d2ae7 100644
--- a/modules/ext.pageTriage.list/components/ListContent.vue
+++ b/modules/ext.pageTriage.list/components/ListContent.vue
@@ -58,6 +58,7 @@ const listItemPropFormatter = ( pageInfo ) => {
 	listItemProps.revCount = parseInt( pageInfo.rev_count );
 	listItemProps.creationDateUTC = pageInfo.creation_date_utc;
 	listItemProps.creatorName = pageInfo.user_name;
+	listItemProps.creatorHidden = pageInfo.creator_hidden;
 	listItemProps.creatorAutoConfirmed = pageInfo.user_autoconfirmed === '1';
 	listItemProps.creatorRegistrationUTC = pageInfo.user_creation_date;
 	listItemProps.creatorUserId = parseInt( pageInfo.user_id );
diff --git a/modules/ext.pageTriage.list/components/ListItem.vue b/modules/ext.pageTriage.list/components/ListItem.vue
index 2a42cd54..45edb46a 100644
--- a/modules/ext.pageTriage.list/components/ListItem.vue
+++ b/modules/ext.pageTriage.list/components/ListItem.vue
@@ -58,14 +58,21 @@
 			</div>
 			<div class="mwe-vue-pt-info-row">
 				<div>
-					<span v-if="creatorName">
+					<span v-if="creatorName || creatorHidden">
 						<creator-byline
+							v-if="creatorName"
 							:creator-name="creatorName"
+							:creator-hidden="creatorHidden"
 							:creator-user-id="creatorUserId"
 							:creator-auto-confirmed="creatorAutoConfirmed"
 							:creator-user-page-exists="creatorUserPageExists"
 							:creator-talk-page-exists="creatorTalkPageExists"
 						></creator-byline>
+						<template v-else>
+							<span class="mwe-pt-history-suppressed">
+								{{ $i18n( 'pagetriage-byline-hidden-username', $i18n( 'rev-deleted-user' ) ) }}
+							</span>
+						</template>
 						<span v-if="creatorUserId > 0">
 							{{ $i18n( 'pagetriage-dot-separator' ).text() }}
 							{{ $i18n( 'pagetriage-editcount', creatorEditCount, creatorRegistrationPretty ).text() }}
@@ -185,6 +192,7 @@ module.exports = {
          */
 		// Creator information tags
 		creatorUserId: { type: Number, required: true },
+		creatorHidden: { type: Boolean, required: true },
 		creatorName: { type: String, required: true },
 		creatorEditCount: { type: Number, required: true },
 		creatorRegistrationUTC: {
@@ -436,4 +444,8 @@ module.exports = {
 .cdx-icon.cdx-info-chip__icon--warning {
 	color: @color-warning;
 }
+.mwe-pt-history-suppressed {
+	text-decoration-line: line-through;
+	text-decoration-style: double;
+}
 </style>
diff --git a/modules/ext.pageTriage.util/models/ext.pageTriage.article.js b/modules/ext.pageTriage.util/models/ext.pageTriage.article.js
index 0c61a0f1..01f0b1b6 100644
--- a/modules/ext.pageTriage.util/models/ext.pageTriage.article.js
+++ b/modules/ext.pageTriage.util/models/ext.pageTriage.article.js
@@ -106,7 +106,12 @@ const Article = Backbone.Model.extend( {
 					article.get( 'creator_user_page_exist' )
 				)
 			);
-			article.set( 'user_contribs_title', article.get( 'creator_contribution_page' ) );
+		} else if ( article.get( 'creator_hidden' ) ) {
+			article.set( 'author_byline_html', mw.msg( 'pagetriage-byline-hidden-username', mw.msg( 'rev-deleted-user' ) ) );
+			article.set(
+				'user_title_url',
+				mw.msg( 'rev-deleted-user' )
+			);
 		}
 
 		// Are there any PageTriage messages on the talk page?
diff --git a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css
index 4d8f89e5..5016727f 100644
--- a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css
+++ b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.css
@@ -123,6 +123,11 @@
 	display: table-cell;
 }
 
+.mwe-pt-history-suppressed {
+	text-decoration-line: line-through;
+	text-decoration-style: double;
+}
+
 .mwe-pt-potential-issues {
 	display: table-cell;
 	text-align: right;
diff --git a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore
index 34cd4944..aa2c3446 100644
--- a/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore
+++ b/modules/ext.pageTriage.views.list/ext.pageTriage.listItem.underscore
@@ -105,7 +105,13 @@
 				<div class="mwe-pt-info-row">
 					<div class="mwe-pt-author">
 					<% if ( typeof( user_name ) !== "undefined" ) { %>
-						<%= author_byline_html %>
+						<% if ( !creator_hidden ) { %>
+							<%= author_byline_html %>
+						<% } else { %>
+							<span class="mwe-pt-history-suppressed">
+								<%= author_byline_html %>
+							</span>
+						<% } %>
 						<!-- user_id is undefined or '0' for IP users -->
 						<% if ( typeof ( user_id ) != 'undefined' && Number( user_id ) !== 0 ) { %>
 							&#xb7;
diff --git a/modules/ext.pageTriage.views.toolbar/ToolView.js b/modules/ext.pageTriage.views.toolbar/ToolView.js
index 74a96746..f2544fec 100644
--- a/modules/ext.pageTriage.views.toolbar/ToolView.js
+++ b/modules/ext.pageTriage.views.toolbar/ToolView.js
@@ -267,6 +267,7 @@ module.exports = Backbone.View.extend( {
 			pageid: mw.config.get( 'wgArticleId' ),
 			title: mw.config.get( 'wgPageName' ),
 			creator: this.model.get( 'user_name' ),
+			creatorHidden: this.model.get( 'creator_hidden' ),
 			reviewed: reviewed
 		}, data );
 	},
diff --git a/modules/ext.pageTriage.views.toolbar/articleInfo.js b/modules/ext.pageTriage.views.toolbar/articleInfo.js
index 9b9ef2af..28eef292 100644
--- a/modules/ext.pageTriage.views.toolbar/articleInfo.js
+++ b/modules/ext.pageTriage.views.toolbar/articleInfo.js
@@ -109,6 +109,8 @@ module.exports = ToolView.extend( {
 			url.toString()
 		);
 
+		const offset = parseInt( mw.user.options.get( 'timecorrection' ).split( '|' )[ 1 ] );
+
 		// creator information
 		if ( this.model.get( 'user_name' ) ) {
 			// show new editor message only if the user is not anonymous and not autoconfirmed
@@ -119,7 +121,6 @@ module.exports = ToolView.extend( {
 				bylineMessage = 'pagetriage-articleinfo-byline';
 			}
 
-			const offset = parseInt( mw.user.options.get( 'timecorrection' ).split( '|' )[ 1 ] );
 			// put it all together in the byline
 			// The following messages are used here:
 			// * pagetriage-articleinfo-byline-new-editor
@@ -150,6 +151,13 @@ module.exports = ToolView.extend( {
 				)
 			).parse();
 			this.model.set( 'articleByline_html', articleByline );
+		} else if ( this.model.get( 'creator_hidden' ) ) {
+			this.model.set( 'articleByline_html', mw.msg( 'pagetriage-articleinfo-byline-hidden-username', moment.utc(
+				this.model.get( 'creation_date_utc' ),
+				'YYYYMMDDHHmmss'
+			).utcOffset( offset ).format(
+				mw.msg( 'pagetriage-info-timestamp-date-format' )
+			), mw.msg( 'rev-deleted-user' ) ) );
 		}
 
 		const stats = [
diff --git a/modules/ext.pageTriage.views.toolbar/articleInfo.underscore b/modules/ext.pageTriage.views.toolbar/articleInfo.underscore
index 9a7ce587..9760e289 100644
--- a/modules/ext.pageTriage.views.toolbar/articleInfo.underscore
+++ b/modules/ext.pageTriage.views.toolbar/articleInfo.underscore
@@ -23,7 +23,13 @@
 	<!-- author info -->
 	<span class="mwe-pt-author">
 		<% if( typeof( user_name ) != 'undefined' ) { %>
-			<%= articleByline_html %>
+			<% if ( !creator_hidden ) { %>
+				<%= articleByline_html %>
+			<% } else { %>
+				<span class="mwe-pt-history-suppressed">
+					<%= articleByline_html %>
+				</span>
+			<% } %>
 			<div>
 				<!-- if user is registered (user_id is 0 for IP users) -->
 				<% if( typeof user_id != 'undefined' && Number( user_id ) !== 0 ) { %>
diff --git a/modules/ext.pageTriage.views.toolbar/mark.js b/modules/ext.pageTriage.views.toolbar/mark.js
index 4d347924..ea221f07 100644
--- a/modules/ext.pageTriage.views.toolbar/mark.js
+++ b/modules/ext.pageTriage.views.toolbar/mark.js
@@ -221,6 +221,7 @@ module.exports = ToolView.extend( {
 			status = this.model.get( 'patrol_status' ) === '0' ? 'reviewed' : 'unreviewed',
 			hasPreviousReviewer = this.model.get( 'ptrp_last_reviewed_by' ) > 0,
 			articleCreator = this.model.get( 'user_name' ),
+			articleCreatorHidden = this.model.get( 'creator_hidden' ),
 			previousReviewer = hasPreviousReviewer ? this.model.get( 'reviewer' ) : '';
 		let noteTarget = articleCreator,
 			notePlaceholder = 'pagetriage-message-for-creator-default-note',
@@ -239,7 +240,7 @@ module.exports = ToolView.extend( {
 			notePlaceholder = 'pagetriage-message-for-creator-default-note';
 		}
 
-		if ( mw.config.get( 'wgUserName' ) === articleCreator ) {
+		if ( mw.config.get( 'wgUserName' ) === articleCreator || articleCreatorHidden ) {
 			numRecipients--;
 			noteTarget = previousReviewer;
 			noteRecipientRole = 'reviewer';
diff --git a/modules/ext.pageTriage.views.toolbar/tags.js b/modules/ext.pageTriage.views.toolbar/tags.js
index 45ebfc61..b1d9a896 100644
--- a/modules/ext.pageTriage.views.toolbar/tags.js
+++ b/modules/ext.pageTriage.views.toolbar/tags.js
@@ -308,7 +308,7 @@ module.exports = ToolView.extend( {
 		if ( this.selectedTagCount > 0 ) {
 			$( '#mwe-pt-tag-submit-button' ).button( 'enable' );
 			$( '#mwe-pt-checkbox-mark-reviewed-wrapper' ).show();
-			if ( mw.config.get( 'wgUserName' ) !== this.model.get( 'user_name' ) ) {
+			if ( mw.config.get( 'wgUserName' ) !== this.model.get( 'user_name' ) && !this.model.get( 'creator_hidden' ) ) {
 				$( '#mwe-pt-tag-note' ).show();
 			}
 		} else {
diff --git a/modules/ext.pageTriage.views.toolbar/wikiLove.js b/modules/ext.pageTriage.views.toolbar/wikiLove.js
index f493b069..22fe7d0a 100644
--- a/modules/ext.pageTriage.views.toolbar/wikiLove.js
+++ b/modules/ext.pageTriage.views.toolbar/wikiLove.js
@@ -26,11 +26,14 @@ module.exports = ToolView.extend( {
 	render: function () {
 		// get the article's creator
 		const creator = this.model.get( 'user_name' );
+		const creatorHidden = this.model.get( 'creator_hidden' );
 
 		// get the last 20 editors of the article
 		const contributorArray = [];
 		this.model.revisions.each( function ( revision ) {
-			contributorArray.push( revision.get( 'user' ) );
+			if ( typeof ( revision.get( 'userhidden' ) ) === 'undefined' ) {
+				contributorArray.push( revision.get( 'user' ) );
+			}
 		} );
 
 		// count how many times each editor edited the article
@@ -54,7 +57,7 @@ module.exports = ToolView.extend( {
 		// set the Learn More link URL
 		$( '#mwe-pt-wikilove .mwe-pt-flyout-help-link' ).attr( 'href', this.moduleConfig.helplink );
 
-		if ( mw.user.getName() !== creator ) {
+		if ( mw.user.getName() !== creator && !creatorHidden ) {
 			// add the creator info to the top of the list
 			$( '#mwe-pt-article-contributor-list' ).append(
 				'<input type="checkbox" class="mwe-pt-recipient-checkbox" value="' + _.escape( creator ) + '"/>' +
-- 
2.42.0

We don't have the "Hide username from edits and lists" option in our local block pages; so we can't reproduce the issue until we have that sorted.

Are you testing in MediaWiki-Docker? Or MediaWiki-Vagrant? Suppress/delete functionality should be within MediaWiki core by default. You'd probably just want to create a suppress group and add a test user to it or just add various rights to the sysop group within LocalSettings.php. Here's an example of enwiki's suppress group config via shell.php:

> print_r( $wgGroupPermissions['suppress'] );
Array
(
    [browsearchive] => 1
    [deletedhistory] => 1
    [deletedtext] => 1
    [abusefilter-view-private] => 1
    [deleterevision] => 1
    [deletelogentry] => 1
    [hideuser] => 1
    [suppressrevision] => 1
    [suppressionlog] => 1
    [abusefilter-hide-log] => 1
    [abusefilter-hidden-log] => 1
    [viewsuppressed] => 1
    [oathauth-enable] => 1
)

You'd likely really only need to add hideuser to get the various controls to which I think you're referring.

We're using MediaWiki Docker. Let me try adding the suppress group to see if I can make that configuration to show up.

I did look into doing this, but that would require having the unescaped HTML be a part of the message, which isn't considered a best practice (AFAIK, since it could lead to XSS attack vectors) or splitting up the message in such a way that it wasn't translatable (which @taavi mentioned).

I don't see many uses of line-through in the code base, and where there are instances, it is to show something is deprecated. You can also see the functionality in Special:UnwatchedPages, but I don't think unwatching a page (with the capability to undo it) is the same as showing a removed user. In conclusion, I think you should remove the line-though CSS.

Can you also rebase this patch with the latest changes in the master branch?

I did look into doing this, but that would require having the unescaped HTML be a part of the message, which isn't considered a best practice (AFAIK, since it could lead to XSS attack vectors) or splitting up the message in such a way that it wasn't translatable (which @taavi mentioned).

I don't see many uses of line-through in the code base, and where there are instances, it is to show something is deprecated. You can also see the functionality in Special:UnwatchedPages, but I don't think unwatching a page (with the capability to undo it) is the same as showing a removed user. In conclusion, I think you should remove the line-though CSS.

So, I'm unsure about this, my understanding was that line-through was the way that redacted information was shown to the user in MediaWiki?

Can you also rebase this patch with the latest changes in the master branch?

The latest one should be rebased on top of master

I did look into doing this, but that would require having the unescaped HTML be a part of the message, which isn't considered a best practice (AFAIK, since it could lead to XSS attack vectors) or splitting up the message in such a way that it wasn't translatable (which @taavi mentioned).

We have prior art to look at with the other logs in core; eg. revision history:

image.png (703×1 px, 118 KB)

the message key is rev-deleted-user; I don't think you need to create a new message or rework the markup. I think you can simply use that message from core instead of the username in the byline.

I don't see many uses of line-through in the code base, and where there are instances, it is to show something is deprecated. You can also see the functionality in Special:UnwatchedPages, but I don't think unwatching a page (with the capability to undo it) is the same as showing a removed user. In conclusion, I think you should remove the line-though CSS.

So, I'm unsure about this, my understanding was that line-through was the way that redacted information was shown to the user in MediaWiki?

Can you also rebase this patch with the latest changes in the master branch?

The latest one should be rebased on top of master

Yep, the parent of your commit is now from yesterday; thanks!

We don't have the "Hide username from edits and lists" option in our local block pages; so we can't reproduce the issue until we have that sorted.

Are you testing in MediaWiki-Docker? Or MediaWiki-Vagrant? Suppress/delete functionality should be within MediaWiki core by default. You'd probably just want to create a suppress group and add a test user to it or just add various rights to the sysop group within LocalSettings.php. Here's an example of enwiki's suppress group config via shell.php:

> print_r( $wgGroupPermissions['suppress'] );
Array
(
    [browsearchive] => 1
    [deletedhistory] => 1
    [deletedtext] => 1
    [abusefilter-view-private] => 1
    [deleterevision] => 1
    [deletelogentry] => 1
    [hideuser] => 1
    [suppressrevision] => 1
    [suppressionlog] => 1
    [abusefilter-hide-log] => 1
    [abusefilter-hidden-log] => 1
    [viewsuppressed] => 1
    [oathauth-enable] => 1
)

You'd likely really only need to add hideuser to get the various controls to which I think you're referring.

Thanks for this; that did the trick and saved me time trying to grok the docs. :-)

@Soda, Because this change touches things in multiple places. I'm really struggling to give actionable feedback without commenting on a diff. I'm going to make an annotated patch that has the same parent commit as this patch and contains my proposed changes. If you apply the commit to a new branch locally, you should be able to diff it against your local branch to make sense of it.

Okay, here's my patch that addresess the vue interface as an example. I haven't touched the backbone side.
Note that we're no longer showing information about edit history. In core changelists, when a user is suppressed, references to their contributions are removed as well, so I think we should not show contribution information on this public page either. I think this is a simpler approach and more appropriate for a security fix.

@Soda, Because this change touches things in multiple places. I'm really struggling to give actionable feedback without commenting on a diff. I'm going to make an annotated patch that has the same parent commit as this patch and contains my proposed changes. If you apply the commit to a new branch locally, you should be able to diff it against your local branch to make sense of it.

Sure

Okay, here's my patch that addresess the vue interface as an example. I haven't touched the backbone side.
Note that we're no longer showing information about edit history. In core changelists, when a user is suppressed, references to their contributions are removed as well, so I think we should not show contribution information on this public page either. I think this is a simpler approach and more appropriate for a security fix.

The patch you uploaded seems empty, but I think that approach would also work imo

Oops. I diffed against my own commit instead of the parent. Let's try this again:

I've updated the patch based on your comments, to have the backbone.js version reflect the same behaviour.


Wrt to the articleInfo section in the toolbar, I elected to remove the rev-deleted-user message completely and just have the message display the date of creation. (Since we already display the context of a redacted edit in the history below)

Screenshot from 2023-09-14 01-41-52.png (337×523 px, 38 KB)

That being said, I'm open to other ways of implementing this.

Thank you for all of your work on this so far. This is looking pretty good. I see two issues left:

  • in modules/ext.pageTriage.views.toolbar/articleInfo.underscore, your changes look like they are no longer needed. eg.:
			<% if ( !creator_hidden ) { %>
				<%= articleByline_html %>
			<% } else { %>
				<%= articleByline_html %>
			<% } %>

could be simplified back to:

			<%= articleByline_html %>
  • You're still defining and using mwe-pt-history-suppressed; I think this could be handled with the built in classes which some markup changes in the articleInfoHistory template

I don't think either of these is blocking; if you don't want to get into reworking the template especially, please add a \@TODO or \@FIXME comment and I'll take a pass at it after this is deployed and we can use the normal workflow again.

Thank you for all of your work on this so far. This is looking pretty good. I see two issues left:

  • in modules/ext.pageTriage.views.toolbar/articleInfo.underscore, your changes look like they are no longer needed. eg.:
			<% if ( !creator_hidden ) { %>
				<%= articleByline_html %>
			<% } else { %>
				<%= articleByline_html %>
			<% } %>

could be simplified back to:

			<%= articleByline_html %>

Fixed :)

  • You're still defining and using mwe-pt-history-suppressed; I think this could be handled with the built in classes which some markup changes in the articleInfoHistory template

I don't think either of these is blocking; if you don't want to get into reworking the template especially, please add a \@TODO or \@FIXME comment and I'll take a pass at it after this is deployed and we can use the normal workflow again.

I did clean this up a fair bit, but it seems like we still need to do some work, since there seems to be some issues issues with suppressed being triggered over userhidden and the history-deleted styles not showing up. I've added a todo for this :)

Thank you for all of your work on this so far. This is looking pretty good. I see two issues left:

  • in modules/ext.pageTriage.views.toolbar/articleInfo.underscore, your changes look like they are no longer needed. eg.:
			<% if ( !creator_hidden ) { %>
				<%= articleByline_html %>
			<% } else { %>
				<%= articleByline_html %>
			<% } %>

could be simplified back to:

			<%= articleByline_html %>

Fixed :)

  • You're still defining and using mwe-pt-history-suppressed; I think this could be handled with the built in classes which some markup changes in the articleInfoHistory template

I don't think either of these is blocking; if you don't want to get into reworking the template especially, please add a \@TODO or \@FIXME comment and I'll take a pass at it after this is deployed and we can use the normal workflow again.

I did clean this up a fair bit, but it seems like we still need to do some work, since there seems to be some issues issues with suppressed being triggered over userhidden and the history-deleted styles not showing up. I've added a todo for this :)

CR + 2
Thank you for both identifying this and patching it! Your contributions are very much appreciated!

CR + 2
Thank you for both identifying this and patching it! Your contributions are very much appreciated!

Great. The Security-Team can get this deployed during next Monday's (2023-09-18) security deployment window.

CR + 2
Thank you for both identifying this and patching it! Your contributions are very much appreciated!

Great. The Security-Team can get this deployed during next Monday's (2023-09-18) security deployment window.

I'm happy to make make self available for testing the deployment if that would be helpful.

I'm happy to make make self available for testing the deployment if that would be helpful.

That'd be great. IRC or Slack?

I'm happy to make make self available for testing the deployment if that would be helpful.

That'd be great. IRC or Slack?

Slack, please!

Thank you for all of your work on this so far. This is looking pretty good. I see two issues left:

  • in modules/ext.pageTriage.views.toolbar/articleInfo.underscore, your changes look like they are no longer needed. eg.:
			<% if ( !creator_hidden ) { %>
				<%= articleByline_html %>
			<% } else { %>
				<%= articleByline_html %>
			<% } %>

could be simplified back to:

			<%= articleByline_html %>

Fixed :)

  • You're still defining and using mwe-pt-history-suppressed; I think this could be handled with the built in classes which some markup changes in the articleInfoHistory template

I don't think either of these is blocking; if you don't want to get into reworking the template especially, please add a \@TODO or \@FIXME comment and I'll take a pass at it after this is deployed and we can use the normal workflow again.

I did clean this up a fair bit, but it seems like we still need to do some work, since there seems to be some issues issues with suppressed being triggered over userhidden and the history-deleted styles not showing up. I've added a todo for this :)

Deployed and then tested by @jsn.sherman and everything looks good. Look out for this in the supplemental release coming out at the end of the month

Hi, this patch currently has a merge conflict with the latest extension code in master: https://releases-jenkins.wikimedia.org/job/Branch%20cut%20pretest%20patches/68/console

We're going to need a rebase/fix in order to be able to roll out this week's train. I've created a directory on the deployment server for the new version at /srv/patches/1.41.0-wmf.28

We're going to need a rebase/fix in order to be able to roll out this week's train. I've created a directory on the deployment server for the new version at /srv/patches/1.41.0-wmf.28

Rebased patch:


There was a minor, one-line conflict in includes/SpecialNewPagesFeed.php from c960120.

Change 960676 had a related patch set uploaded (by Mstyles; author: Sohom Datta):

[mediawiki/extensions/PageTriage@master] SECURITY: Don't expose usernames if user is hidden

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

Change 960676 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] SECURITY: Don't expose usernames if user is hidden

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

Mstyles renamed this task from pagetriagelist API leaks suppressed usernames to CVE-2023-45369: pagetriagelist API leaks suppressed usernames.Oct 10 2023, 3:59 PM
Mstyles changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 10 2023, 5:07 PM
Mstyles changed the edit policy from "Custom Policy" to "All Users".