auth: Disable irrelevant account creation fields for temp users

Why:

- When signing up for an account, temporary users are currently forced
  to provide a reason for creating an account, and also have the option
  to send a temporary password to an email address.
- Neither of these options are useful for temporary users wanting to
  create a full user account.

What:

- Don't show these two form fields on Special:CreateAccount for temporary users.
- Add a functional test for the temporary user account creation flow.

Bug: T328718
Change-Id: Ie545857f7647be89d9462ec2b0def48690f0a2bf

includes/auth/AuthManager.php

@@ -2509,7 +2509,13 @@ class AuthManager implements LoggerAwareInterface {
 			case self::ACTION_CREATE:
 				$reqs[] = new UsernameAuthenticationRequest;
 				$reqs[] = new UserDataAuthenticationRequest;
-				if ( $options['username'] !== null ) {
+
+				// Registered users should be prompted to provide a rationale for account creations,
+				// except for the case of a temporary user registering a full account (T328718).
+				if (
+					$options['username'] !== null &&
+					!$this->userNameUtils->isTemp( $options['username'] )
+				) {
 					$reqs[] = new CreationReasonAuthenticationRequest;
 					$options['username'] = null; // Don't fill in the username below
 				}

includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php

@@ -114,12 +114,17 @@ class TemporaryPasswordPrimaryAuthenticationProvider
 				return [ TemporaryPasswordAuthenticationRequest::newRandom() ];
 
 			case AuthManager::ACTION_CREATE:
-				if ( isset( $options['username'] ) && $this->emailEnabled ) {
-					// Creating an account for someone else
+				// Allow named users creating a new account to email a temporary password to a given address
+				// in case they are creating an account for somebody else.
+				// This isn't a likely scenario for account creations by anonymous or temporary users
+				// and is therefore disabled for them (T328718).
+				if (
+					isset( $options['username'] ) &&
+					!$this->userNameUtils->isTemp( $options['username'] ) &&
+					$this->emailEnabled
+				) {
 					return [ TemporaryPasswordAuthenticationRequest::newRandom() ];
 				} else {
-					// It's not terribly likely that an anonymous user will
-					// be creating an account for someone else.
 					return [];
 				}
 

tests/phpunit/includes/auth/AuthManagerTest.php

@@ -2,6 +2,7 @@
 
 namespace MediaWiki\Tests\Auth;
 
+use Closure;
 use DatabaseLogEntry;
 use DomainException;
 use DummySessionProvider;
@@ -25,8 +26,10 @@ use MediaWiki\Auth\Hook\AuthManagerVerifyAuthenticationHook;
 use MediaWiki\Auth\Hook\LocalUserCreatedHook;
 use MediaWiki\Auth\Hook\SecuritySensitiveOperationStatusHook;
 use MediaWiki\Auth\Hook\UserLoggedInHook;
+use MediaWiki\Auth\PasswordAuthenticationRequest;
 use MediaWiki\Auth\PrimaryAuthenticationProvider;
 use MediaWiki\Auth\RememberMeAuthenticationRequest;
+use MediaWiki\Auth\TemporaryPasswordAuthenticationRequest;
 use MediaWiki\Auth\UserDataAuthenticationRequest;
 use MediaWiki\Auth\UsernameAuthenticationRequest;
 use MediaWiki\Block\BlockManager;
@@ -4236,4 +4239,93 @@ class AuthManagerTest extends MediaWikiIntegrationTestCase {
 		$this->assertSame( $context->getRequest()->getSession()->getUser()->getName(), $newSessionUser->getName() );
 		$this->assertSame( $context->getRequest()->getSession()->getUser()->getName(), $context->getUser()->getName() );
 	}
+
+	/**
+	 * @dataProvider provideAccountCreationAuthenticationRequestTestCases
+	 *
+	 * @param Closure $userProvider Closure returning the user performing the account creation
+	 * @param string[] $expectedReqsByLevel Map of expected auth request classes keyed by requirement level
+	 */
+	public function testDefaultAccountCreationAuthenticationRequests(
+		Closure $userProvider,
+		array $expectedReqsByLevel
+	): void {
+		// Test the default primary and secondary authentication providers
+		// irrespective of any potentially conflicting local configuration.
+		$authConfig = $this->getServiceContainer()
+			->getConfigSchema()
+			->getDefaultFor( MainConfigNames::AuthManagerAutoConfig );
+
+		$this->overrideConfigValues( [
+			MainConfigNames::AuthManagerConfig => null,
+			MainConfigNames::AuthManagerAutoConfig => $authConfig
+		] );
+
+		$authManager = $this->getServiceContainer()->getAuthManager();
+		$userProvider = $userProvider->bindTo( $this );
+
+		$reqs = $authManager->getAuthenticationRequests( AuthManager::ACTION_CREATE, $userProvider() );
+
+		$reqsByLevel = [];
+		foreach ( $reqs as $req ) {
+			$reqsByLevel[$req->required][] = get_class( $req );
+		}
+
+		foreach ( $expectedReqsByLevel as $level => $expectedReqs ) {
+			$reqs = $reqsByLevel[$level] ?? [];
+			sort( $reqs );
+			sort( $expectedReqs );
+
+			$this->assertSame( $expectedReqs, $reqs );
+		}
+	}
+
+	public static function provideAccountCreationAuthenticationRequestTestCases(): iterable {
+		// phpcs:disable Squiz.Scope.StaticThisUsage.Found
+		yield 'account creation on behalf of anonymous user' => [
+			fn (): User => $this->getServiceContainer()->getUserFactory()->newAnonymous( '127.0.0.1' ),
+			[
+				AuthenticationRequest::OPTIONAL => [],
+				AuthenticationRequest::REQUIRED => [
+					UserDataAuthenticationRequest::class,
+					UsernameAuthenticationRequest::class
+				],
+				AuthenticationRequest::PRIMARY_REQUIRED => [ PasswordAuthenticationRequest::class ]
+			]
+		];
+
+		yield 'account creation on behalf of temporary user' => [
+			function (): User {
+				$req = new FauxRequest();
+				return $this->getServiceContainer()
+					->getTempUserCreator()
+					->create( null, $req )
+					->getUser();
+			},
+			[
+				AuthenticationRequest::OPTIONAL => [],
+				AuthenticationRequest::REQUIRED => [
+					UserDataAuthenticationRequest::class,
+					UsernameAuthenticationRequest::class
+				],
+				AuthenticationRequest::PRIMARY_REQUIRED => [ PasswordAuthenticationRequest::class ]
+			]
+		];
+
+		yield 'account creation on behalf of registered user' => [
+			fn (): User => $this->getTestUser()->getUser(),
+			[
+				AuthenticationRequest::OPTIONAL => [ CreationReasonAuthenticationRequest::class ],
+				AuthenticationRequest::REQUIRED => [
+					UserDataAuthenticationRequest::class,
+					UsernameAuthenticationRequest::class
+				],
+				AuthenticationRequest::PRIMARY_REQUIRED => [
+					PasswordAuthenticationRequest::class,
+					TemporaryPasswordAuthenticationRequest::class
+				]
+			]
+		];
+		// phpcs:enable
+	}
 }

tests/phpunit/includes/auth/TemporaryPasswordPrimaryAuthenticationProviderTest.php

@@ -340,7 +340,7 @@ class TemporaryPasswordPrimaryAuthenticationProviderTest extends MediaWikiIntegr
 		];
 
 		yield 'signup attempt as temporary user' => [
-			AuthManager::ACTION_CREATE, true, true, [ new TemporaryPasswordAuthenticationRequest( 'random' ) ]
+			AuthManager::ACTION_CREATE, true, true, []
 		];
 
 		yield 'account linking attempt as anonymous user' => [

tests/phpunit/includes/specials/SpecialCreateAccountTest.php

@@ -1,9 +1,13 @@
 <?php
 
+use HtmlFormatter\HtmlFormatter;
 use MediaWiki\Auth\AbstractPreAuthenticationProvider;
 use MediaWiki\Auth\AuthenticationRequest;
+use MediaWiki\Context\DerivativeContext;
+use MediaWiki\Context\IContextSource;
 use MediaWiki\Context\RequestContext;
 use MediaWiki\MainConfigNames;
+use MediaWiki\Request\FauxRequest;
 use MediaWiki\Specials\SpecialCreateAccount;
 
 /**
@@ -14,13 +18,14 @@ class SpecialCreateAccountTest extends SpecialPageTestBase {
 	/**
 	 * @inheritDoc
 	 */
-	protected function newSpecialPage() {
+	protected function newSpecialPage( ?IContextSource $context = null ) {
 		$services = $this->getServiceContainer();
-		$context = RequestContext::getMain();
+		$context ??= RequestContext::getMain();
 		$page = new SpecialCreateAccount(
 			$services->getAuthManager(),
 			$services->getFormatterFactory()
 		);
+		$page->setContext( $context );
 		$context->setTitle( $page->getPageTitle() );
 		return $page;
 	}
@@ -51,6 +56,54 @@ class SpecialCreateAccountTest extends SpecialPageTestBase {
 			$html
 		);
 	}
+
+	public function testShouldShowTemporaryPasswordAndCreationReasonFieldsForRegisteredUser(): void {
+		$user = $this->getTestUser()->getUser();
+
+		$context = new DerivativeContext( RequestContext::getMain() );
+		$context->setUser( $user );
+
+		$specialPage = $this->newSpecialPage( $context );
+		$specialPage->execute( null );
+		$doc = self::getOutputHtml( $specialPage );
+
+		$this->assertNotNull( $doc->getElementById( 'wpReason' ) );
+		$this->assertNotNull( $doc->getElementById( 'wpCreateaccountMail' ) );
+	}
+
+	public function testShouldNotShowTemporaryPasswordAndCreationReasonFieldsForTempUser(): void {
+		$req = new FauxRequest();
+		$tempUser = $this->getServiceContainer()
+			->getTempUserCreator()
+			->create( null, $req )
+			->getUser();
+
+		$context = new DerivativeContext( RequestContext::getMain() );
+		$context->setUser( $tempUser );
+
+		$specialPage = $this->newSpecialPage( $context );
+		$specialPage->execute( null );
+		$doc = self::getOutputHtml( $specialPage );
+
+		$this->assertNull(
+			$doc->getElementById( 'wpReason' ),
+			'Temporary users should not have to provide a reason for their account creation (T328718)'
+		);
+		$this->assertNull(
+			$doc->getElementById( 'wpCreateaccountMail' ),
+			'Temporary users should not have the option to have a temporary password sent on signup (T328718)'
+		);
+	}
+
+	/**
+	 * Convenience function to get the parsed DOM of the HTML generated by the given special page.
+	 * @param SpecialPage $page
+	 * @return DOMDocument
+	 */
+	private static function getOutputHtml( SpecialPage $page ): DOMDocument {
+		$html = HtmlFormatter::wrapHTML( $page->getOutput()->getHTML() );
+		return ( new HtmlFormatter( $html ) )->getDoc();
+	}
 }
 
 class MockAuthRequestWithHiddenField extends AuthenticationRequest {

tests/selenium/pageobjects/edit.page.js

@@ -28,6 +28,10 @@ class EditPage extends Page {
 		return $( '#wpPreview' );
 	}
 
+	get tempUserSignUpButton() {
+		return $( '.mw-temp-user-banner-buttons > #pt-createaccount' );
+	}
+
 	async openForEditing( title ) {
 		await super.openTitle( title, { action: 'submit', vehidebetadialog: 1, hidewelcomedialog: 1 } );
 		// Compatibility with CodeMirror extension (T324879)
@@ -55,6 +59,16 @@ class EditPage extends Page {
 		await this.content.setValue( content );
 		await this.save.click();
 	}
+
+	/**
+	 * Navigate to Special:CreateAccount via the banner links if logged in as a temporary user.
+	 *
+	 * @return {Promise<void>}
+	 */
+	async openCreateAccountPageAsTempUser() {
+		await this.tempUserSignUpButton.waitForDisplayed();
+		await this.tempUserSignUpButton.click();
+	}
 }
 
 module.exports = new EditPage();

tests/selenium/specs/user.js

@@ -5,6 +5,7 @@
 
 const assert = require( 'assert' );
 const CreateAccountPage = require( 'wdio-mediawiki/CreateAccountPage' );
+const EditPage = require( '../pageobjects/edit.page' );
 const UserLoginPage = require( 'wdio-mediawiki/LoginPage' );
 const Api = require( 'wdio-mediawiki/Api' );
 const Util = require( 'wdio-mediawiki/Util' );
@@ -41,4 +42,56 @@ describe( 'User', () => {
 		const actualUsername = await browser.execute( () => mw.config.get( 'wgUserName' ) );
 		assert.strictEqual( await actualUsername, username );
 	} );
+
+	it( 'named user should see extra signup form fields when creating an account', async () => {
+		await Api.createAccount( bot, username, password );
+		await UserLoginPage.login( username, password );
+
+		await CreateAccountPage.open();
+
+		assert.ok( await CreateAccountPage.username.isExisting() );
+		assert.ok( await CreateAccountPage.password.isExisting() );
+		assert.ok(
+			await CreateAccountPage.tempPasswordInput.isExisting(),
+			'Named users should have the option to have a temporary password sent on signup (T328718)'
+		);
+		assert.ok(
+			await CreateAccountPage.reasonInput.isExisting(),
+			'Named users should have to provide a reason for their account creation (T328718)'
+		);
+	} );
+
+	it( 'temporary user should not see signup form fields relevant to named users', async () => {
+		const pageTitle = Util.getTestString( 'TempUserSignup-TestPage-' );
+		const pageText = Util.getTestString();
+
+		await EditPage.edit( pageTitle, pageText );
+		await EditPage.openCreateAccountPageAsTempUser();
+
+		assert.ok( await CreateAccountPage.username.isExisting() );
+		assert.ok( await CreateAccountPage.password.isExisting() );
+		assert.ok(
+			!await CreateAccountPage.tempPasswordInput.isExisting(),
+			'Temporary users should not have the option to have a temporary password sent on signup (T328718)'
+		);
+		assert.ok(
+			!await CreateAccountPage.reasonInput.isExisting(),
+			'Temporary users should not have to provide a reason for their account creation (T328718)'
+		);
+	} );
+
+	// NOTE: This test can't run parallel with other account creation tests (T199393)
+	it( 'temporary user should be able to create account', async () => {
+		const pageTitle = Util.getTestString( 'TempUserSignup-TestPage-' );
+		const pageText = Util.getTestString();
+
+		await EditPage.edit( pageTitle, pageText );
+		await EditPage.openCreateAccountPageAsTempUser();
+
+		await CreateAccountPage.submitForm( username, password );
+
+		const actualUsername = browser.execute( () => mw.config.get( 'wgUserName' ) );
+		assert.strictEqual( await actualUsername, username );
+		assert.strictEqual( await CreateAccountPage.heading.getText(), `Welcome, ${ username }!` );
+	} );
 } );

tests/selenium/wdio-mediawiki/CreateAccountPage.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const Page = require( './Page' );
+const Util = require( 'wdio-mediawiki/Util' );
 
 class CreateAccountPage extends Page {
 	get username() {
@@ -23,12 +24,41 @@ class CreateAccountPage extends Page {
 		return $( '#firstHeading' );
 	}
 
+	get tempPasswordInput() {
+		return $( '#wpCreateaccountMail' );
+	}
+
+	get reasonInput() {
+		return $( '#wpReason' );
+	}
+
 	open() {
 		super.openTitle( 'Special:CreateAccount' );
 	}
 
+	/**
+	 * Navigate to Special:CreateAccount, then fill out and submit the account creation form.
+	 *
+	 * @param {string} username
+	 * @param {string} password
+	 * @return {Promise<void>}
+	 */
 	async createAccount( username, password ) {
 		await this.open();
+		await this.submitForm( username, password );
+	}
+
+	/**
+	 * Fill out and submit the account creation form on Special:CreateAccount.
+	 * The browser is assumed to have already navigated to this page.
+	 *
+	 * @param {string} username
+	 * @param {string} password
+	 * @return {Promise<void>}
+	 */
+	async submitForm( username, password ) {
+		await Util.waitForModuleState( 'mediawiki.special.createaccount', 'ready', 10000 );
+
 		await this.username.setValue( username );
 		await this.password.setValue( password );
 		await this.confirmPassword.setValue( password );

tests/selenium/wdio-mediawiki/wdio-defaults.conf.js

@@ -66,7 +66,10 @@ exports.config = {
 				'--enable-automation',
 				...( process.env.DISPLAY ? [] : [ '--headless' ] ),
 				// Chrome sandbox does not work in Docker
-				...( fs.existsSync( '/.dockerenv' ) ? [ '--no-sandbox' ] : [] )
+				...( fs.existsSync( '/.dockerenv' ) ? [ '--no-sandbox' ] : [] ),
+				// Workaround inputs not working consistently post-navigation on Chrome 90
+				// https://issuetracker.google.com/issues/42322798
+				'--allow-pre-commit-input'
 			]
 		}
 	} ],