From b212197020c6c739bc4e6c0bfceb950a5c7dbdfa Mon Sep 17 00:00:00 2001
From: =?utf8?q?Gerg=C5=91=20Tisza?= <gtisza@wikimedia.org>
Date: Sat, 16 Apr 2016 17:36:33 +0000
Subject: [PATCH] Reject usernames with namespace or interwiki prefixes

User::getCanonicalName silently stripped namepace/iw prefixes,
resulting in weird behavior (e.g. entering 'Template:Foo' in the
registration form username field was accepted, but user 'Foo' was
created). Explicitly reject such names instead.

Also remove leading whitespace from test cases testing for something
else.

Bug: T94656
Change-Id: Idfb22f998cb069fca24abcf5809afd6e0324b0ae
---
 includes/user/User.php                   |  4 +-
 tests/phpunit/includes/user/UserTest.php | 48 ++++++++++++++++--------
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/includes/user/User.php b/includes/user/User.php
index 04eba97b31..c1e096b93b 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -1070,9 +1070,9 @@ class User implements IDBAccessObject {
 		// Clean up name according to title rules,
 		// but only when validation is requested (bug 12654)
 		$t = ( $validate !== false ) ?
-			Title::newFromText( $name ) : Title::makeTitle( NS_USER, $name );
+			Title::newFromText( $name, NS_USER ) : Title::makeTitle( NS_USER, $name );
 		// Check for invalid titles
-		if ( is_null( $t ) ) {
+		if ( is_null( $t ) || $t->getNamespace() !== NS_USER || $t->isExternal() ) {
 			return false;
 		}
 
diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index 88bb328d2f..c9b6929e7a 100644
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -342,30 +342,46 @@ class UserTest extends MediaWikiTestCase {
 	 * @covers User::getCanonicalName()
 	 * @dataProvider provideGetCanonicalName
 	 */
-	public function testGetCanonicalName( $name, $expectedArray, $msg ) {
+	public function testGetCanonicalName( $name, $expectedArray ) {
+		// fake interwiki map for the 'Interwiki prefix' testcase
+		$this->mergeMwGlobalArrayValue( 'wgHooks', [
+			'InterwikiLoadPrefix' => [
+				function ( $prefix, &$iwdata ) {
+					if ( $prefix === 'interwiki' ) {
+						$iwdata = [
+							'iw_url' => 'http://example.com/',
+							'iw_local' => 0,
+							'iw_trans' => 0,
+						];
+						return false;
+					}
+				},
+			],
+		] );
+
 		foreach ( $expectedArray as $validate => $expected ) {
 			$this->assertEquals(
 				$expected,
-				User::getCanonicalName( $name, $validate === 'false' ? false : $validate ),
-				$msg . ' (' . $validate . ')'
-			);
+				User::getCanonicalName( $name, $validate === 'false' ? false : $validate ), $validate );
 		}
 	}
 
 	public static function provideGetCanonicalName() {
 		return [
-			[ ' Trailing space ', [ 'creatable' => 'Trailing space' ], 'Trailing spaces' ],
-			// @todo FIXME: Maybe the creatable name should be 'Talk:Username' or false to reject?
-			[ 'Talk:Username', [ 'creatable' => 'Username', 'usable' => 'Username',
-				'valid' => 'Username', 'false' => 'Talk:Username' ], 'Namespace prefix' ],
-			[ ' name with # hash', [ 'creatable' => false, 'usable' => false ], 'With hash' ],
-			[ 'Multi  spaces', [ 'creatable' => 'Multi spaces',
-				'usable' => 'Multi spaces' ], 'Multi spaces' ],
-			[ 'lowercase', [ 'creatable' => 'Lowercase' ], 'Lowercase' ],
-			[ 'in[]valid', [ 'creatable' => false, 'usable' => false, 'valid' => false,
-				'false' => 'In[]valid' ], 'Invalid' ],
-			[ 'with / slash', [ 'creatable' => false, 'usable' => false, 'valid' => false,
-				'false' => 'With / slash' ], 'With slash' ],
+			'Leading space' => [ ' Leading space', [ 'creatable' => 'Leading space' ] ],
+			'Trailing space ' => [ 'Trailing space ', [ 'creatable' => 'Trailing space' ] ],
+			'Namespace prefix' => [ 'Talk:Username', [ 'creatable' => false, 'usable' => false,
+				'valid' => false, 'false' => 'Talk:Username' ] ],
+			'Interwiki prefix' => [ 'interwiki:Username', [ 'creatable' => false, 'usable' => false,
+				'valid' => false, 'false' => 'Interwiki:Username' ] ],
+			'With hash' => [ 'name with # hash', [ 'creatable' => false, 'usable' => false ] ],
+			'Multi spaces' => [ 'Multi  spaces', [ 'creatable' => 'Multi spaces',
+				'usable' => 'Multi spaces' ] ],
+			'Lowercase' => [ 'lowercase', [ 'creatable' => 'Lowercase' ] ],
+			'Invalid character' => [ 'in[]valid', [ 'creatable' => false, 'usable' => false,
+				'valid' => false, 'false' => 'In[]valid' ] ],
+			'With slash' => [ 'with / slash', [ 'creatable' => false, 'usable' => false, 'valid' => false,
+				'false' => 'With / slash' ] ],
 		];
 	}
 
-- 
2.20.1