Temporary accounts: Create user on edit save attempts

Why:

- Certain hooks and constraints that deny edits (SpamBlacklist,
  AbuseFilter) generate manual log entries; these logs need to
  reference an actor. The actor should be the same across multiple
  failed attempts to save an edit.
- In order to hide the IP address associated with the request,
  we can create a temporary account for every edit attempt, to
  ensure that we associate the request with the log entry.

What:

- Create the temporary account at the beginning of
  `EditPage::internalAttemptSave`. This is a change from the previous
  temporary account creation paradigm, which created an account for a
  successful edit attempt.
- Remove the pageUpdater->isChange check, as we don't have a pageUpdater
  object at time of temp account creation.
- Temporary accounts are also created for no-op changes (pressing
  "Publish" with unchanged wikitext editor contents)
- Note that temporary accounts are *not* created when permission related
  errors are present in the edit attempt (e.g. a user attempts to edit
  from a blocked IP address)

Bug: T359405
Change-Id: Ib6765f828681e70d798363338910a54c7de4ed67

1 files changed, 24 insertions, 29 deletions

diff --git a/includes/editpage/EditPage.php b/includes/editpage/EditPage.php
index 9ae967d9cfc8..ae9efaa9e56b 100644
--- a/includes/editpage/EditPage.php
+++ b/includes/editpage/EditPage.php
@@ -81,7 +81,6 @@ use MediaWiki\Revision\RevisionStoreRecord;
 use MediaWiki\Revision\SlotRecord;
 use MediaWiki\Status\Status;
 use MediaWiki\Storage\EditResult;
-use MediaWiki\Storage\PageUpdater;
 use MediaWiki\Title\Title;
 use MediaWiki\User\ExternalUserNames;
 use MediaWiki\User\Options\UserOptionsLookup;
@@ -804,35 +803,26 @@ class EditPage implements IEditObject {
 	}
 
 	/**
-	 * If automatic user creation is enabled, create the user and adjust the
-	 * PageUpdater so that it has the new user/actor ID.
+	 * If automatic user creation is enabled, create the user.
 	 *
-	 * This is a helper for internalAttemptSave(). The name should have already
-	 * been acquired at this point for PST purposes, but if not, it will be
-	 * acquired here.
+	 * This is a helper for internalAttemptSave().
 	 *
 	 * If the edit is a null edit, the user will not be created.
 	 *
-	 * @param PageUpdater $pageUpdater
 	 * @return Status
 	 */
-	private function createTempUser( PageUpdater $pageUpdater ) {
+	private function createTempUser(): Status {
 		if ( !$this->tempUserCreateActive ) {
 			return Status::newGood();
 		}
-		if ( !$pageUpdater->isChange() ) {
-			$pageUpdater->preventChange();
-			return Status::newGood();
-		}
 		$status = $this->tempUserCreator->create(
-			$this->tempUserName, // acquire if null
+			$this->tempUserName,
 			$this->context->getRequest()
 		);
 		if ( $status->isOK() ) {
 			$this->placeholderTempUser = null;
 			$this->unsavedTempUser = null;
 			$this->savedTempUser = $status->getUser();
-			$pageUpdater->updateAuthor( $status->getUser() );
 			$this->tempUserCreateDone = true;
 		}
 		return $status;
@@ -2018,6 +2008,26 @@ class EditPage implements IEditObject {
 	 * time.
 	 */
 	public function internalAttemptSave( &$result, $markAsBot = false, $markAsMinor = false ) {
+		// If an attempt to acquire a temporary name failed, don't attempt to do anything else.
+		if ( $this->unableToAcquireTempName ) {
+			$status = Status::newFatal( 'temp-user-unable-to-acquire' );
+			$status->value = self::AS_UNABLE_TO_ACQUIRE_TEMP_ACCOUNT;
+			return $status;
+		}
+		// Auto-create the temporary account user, if the feature is enabled.
+		// We create the account before any constraint checks or edit hooks fire, to ensure
+		// that we have an actor and user account that can be used for any logs generated
+		// by the edit attempt, and to ensure continuity in the user experience (if a constraint
+		// denies an edit to a logged-out user, that history should be associated with the
+		// eventually successful account creation)
+		$tempAccountStatus = $this->createTempUser();
+		if ( !$tempAccountStatus->isOK() ) {
+			return $tempAccountStatus;
+		}
+		if ( $tempAccountStatus instanceof CreateStatus ) {
+			$result['savedTempUser'] = $tempAccountStatus->getUser();
+		}
+
 		$useNPPatrol = MediaWikiServices::getInstance()->getMainConfig()->get( MainConfigNames::UseNPPatrol );
 		$useRCPatrol = MediaWikiServices::getInstance()->getMainConfig()->get( MainConfigNames::UseRCPatrol );
 		if ( !$this->getHookRunner()->onEditPage__attemptSave( $this ) ) {
@@ -2041,12 +2051,6 @@ class EditPage implements IEditObject {
 			return $status;
 		}
 
-		if ( $this->unableToAcquireTempName ) {
-			$status = Status::newFatal( 'temp-user-unable-to-acquire' );
-			$status->value = self::AS_UNABLE_TO_ACQUIRE_TEMP_ACCOUNT;
-			return $status;
-		}
-
 		try {
 			# Construct Content object
 			$textbox_content = $this->toEditContent( $this->textbox1 );
@@ -2461,15 +2465,6 @@ class EditPage implements IEditObject {
 		}
 		// END OF MIGRATION TO EDITCONSTRAINT SYSTEM
 
-		// Auto-create the user if that is enabled
-		$status = $this->createTempUser( $pageUpdater );
-		if ( !$status->isOK() ) {
-			return $status;
-		}
-		if ( $status instanceof CreateStatus ) {
-			$result['savedTempUser'] = $status->getUser();
-		}
-
 		if ( $this->undidRev && $this->isUndoClean( $content ) ) {
 			// As the user can change the edit's content before saving, we only mark
 			// "clean" undos as reverts. This is to avoid abuse by marking irrelevant