Fix user_is_email_unique() on case-sensitive DB

core/user_api.php

@@ -193,13 +193,16 @@ function user_update_cache( $p_user_id, $p_field, $p_value ) {
  *
  * @param string $p_field The user object field name to search the cache for.
  * @param mixed  $p_value The field value to look for in the cache.
+ * @param bool   $p_case_sensitive False to perform case-insensitive search; defaults to true.
+ *
  * @return integer|boolean
  */
-function user_search_cache( $p_field, $p_value ) {
+function user_search_cache( $p_field, $p_value, $p_case_sensitive = true ) {
 	global $g_cache_user;
 	if( isset( $g_cache_user ) ) {
+		$t_compare = $p_case_sensitive ? 'strcmp' : 'strcasecmp';
 		foreach( $g_cache_user as $t_user ) {
-			if( $t_user && $t_user[$p_field] == $p_value ) {
+			if( $t_user && 0 == $t_compare( $t_user[$p_field], $p_value ) ) {
 				return $t_user;
 			}
 		}
@@ -288,18 +291,20 @@ function user_is_email_unique( $p_email, $p_user_id = null ) {
 	}
 
 	$p_email = trim( $p_email );
+	// Escape SQL LIKE pattern chars to ensure exact match
+	$p_email = preg_replace( '/([%_])/', '\\\\$1', $p_email );
 
-	db_param_push();
-	if ( $p_user_id === null ) {
-		$t_query = 'SELECT email FROM {user} WHERE email=' . db_param();
-		$t_result = db_query( $t_query, array( $p_email ), 1 );
-	} else {
-		$t_query = 'SELECT email FROM {user} WHERE id<>' . db_param() .
-			' AND email=' . db_param();
-		$t_result = db_query( $t_query, array( $p_user_id, $p_email ), 1 );
+	$t_query = new DbQuery;
+	$t_query->sql( 'SELECT email FROM {user} WHERE '
+		// Case-insensitive like
+		. $t_query->sql_ilike( 'email', $p_email, '\\' )
+	);
+	if( $p_user_id !== null ) {
+		$t_query->append_sql( ' AND id<>' . $t_query->param( $p_user_id ) );
 	}
 
-	return !db_result( $t_result );
+	$t_query->execute();
+	return !$t_query->value();
 }
 
 /**
@@ -720,22 +725,36 @@ function user_get_id_by_name( $p_username, $p_throw = false ) {
 }
 
 /**
- * Get a user id from their email address
+ * Get a user's id from their email address.
+ *
+ * The check is case insensitive.
+ *
+ * This function should not be used when {@see $g_email_ensure_unique} is OFF:
+ * if there are multiple users found matching the given email, the function will
+ * not consider that to be an error, and arbitrarily return one of them (exactly
+ * which one is undetermined).
  *
  * @param string $p_email The email address to retrieve data for.
- * @param boolean $p_throw true to throw exception when not found, false otherwise.
- * @return integer|boolean
+ * @param bool   $p_throw True to throw exception when not found, false otherwise.
+ *
+ * @return int|false User Id or false if the email does not exist.
+ * @throws ClientException
  */
 function user_get_id_by_email( $p_email, $p_throw = false ) {
-	if( $t_user = user_search_cache( 'email', $p_email ) ) {
+	if( $t_user = user_search_cache( 'email', $p_email, false ) ) {
 		return (int)$t_user['id'];
 	}
 
-	db_param_push();
-	$t_query = 'SELECT * FROM {user} WHERE email=' . db_param();
-	$t_result = db_query( $t_query, array( $p_email ) );
+	// Escape SQL LIKE pattern chars to ensure exact match
+	$p_email = preg_replace( '/([%_])/', '\\\\$1', $p_email );
+
+	$t_query = new DbQuery;
+	$t_query->sql( 'SELECT * FROM {user} WHERE '
+		// Case-insensitive like
+		. $t_query->sql_ilike( 'email', $p_email, '\\' )
+	);
+	$t_row = $t_query->fetch();
 
-	$t_row = db_fetch_array( $t_result );
 	if( $t_row ) {
 		user_cache_database_result( $t_row );
 		return (int)$t_row['id'];

tests/Mantis/UserTest.php

@@ -0,0 +1,144 @@
+<?php
+# MantisBT - A PHP based bugtracking system
+
+# MantisBT is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# MantisBT is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with MantisBT.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Test cases for User API within mantis
+ *
+ * @package    Tests
+ * @subpackage UserAPI
+ * @copyright  Copyright 2023  MantisBT Team - mantisbt-dev@lists.sourceforge.net
+ * @link       http://www.mantisbt.org
+ *
+ * @noinspection PhpIllegalPsrClassPathInspection
+ */
+
+# Includes
+use Mantis\Exceptions\ClientException;
+
+require_once 'MantisCoreBase.php';
+
+/**
+ * PHPUnit tests for User API
+ */
+class MantisUserApiTest extends MantisCoreBase {
+
+	const TEST_EMAIL = 'test@uniqueness.test';
+
+	protected static $user_id;
+
+	public static function setUpBeforeClass() {
+		parent::setUpBeforeClass();
+
+		$t_cookie = user_create(
+			'User' . rand(),
+			'password',
+			self::TEST_EMAIL
+		);
+		/** @noinspection PhpUnhandledExceptionInspection */
+		self::$user_id = user_get_id_by_cookie( $t_cookie );
+	}
+
+	public static function tearDownAfterClass() {
+		user_delete( self::$user_id );
+	}
+
+
+	/**
+	 * Tests user_is_email_unique()
+	 *
+	 * @dataProvider providerEmailUnique
+	 * @param string $p_email
+	 * @param int    $p_user_id
+	 * @param bool   $p_unique  Expected result.
+	 */
+	public function testEmailUnique( $p_email, $p_user_id, $p_unique ) {
+		if( $p_user_id == -1 ) {
+			$p_user_id = $this::$user_id;
+		}
+		$this->assertEquals( user_is_email_unique( $p_email, $p_user_id ), $p_unique );
+	}
+
+	/**
+	 * Data provider for testEmailUnique().
+	 *
+	 * Set user_id to `-1` to use the id of the test user created in
+	 * setUpBeforeClass(). This hack is needed because PHPUnit initializes the
+	 * data provider before the setup method has created the test user account.
+	 *
+	 * @return array [email_address, user_id, unique]
+	 */
+	public function providerEmailUnique() {
+		return [
+			"Existing email, new user"
+				=> array( self::TEST_EMAIL, null, false ),
+			"Existing email, matching user"
+				=> array( self::TEST_EMAIL, -1, true ),
+			"Existing email, other user"
+				=> array( self::TEST_EMAIL, 1, false ),
+			"Existing email with different case"
+				=> array( ucfirst(self::TEST_EMAIL), null, false ),
+			"Email matching SQL LIKE pattern"
+				=> array( 't_st@uniqueness.test', null, true ),
+			"Non-existing email"
+				=> array( 'unique@uniqueness.test', null, true ),
+		];
+	}
+
+	/**
+	 * Tests user_get_id_by_email()
+	 *
+	 * @noinspection PhpUnhandledExceptionInspection
+	 */
+	public function testGetIdByEmail() {
+		$t_user_id = $this::$user_id;
+		$t_email_with_case_variation = ucfirst( self::TEST_EMAIL );
+
+		$this->assertEquals( $t_user_id,
+			user_get_id_by_email( self::TEST_EMAIL ),
+			"User email found with exact case"
+			 );
+		$this->assertEquals( $t_user_id,
+			user_get_id_by_email( $t_email_with_case_variation ),
+			"User email found with different case"
+		);
+
+		// Allow non-unique emails and create a new user with duplicate email
+		config_set_global( 'email_ensure_unique', false );
+		$t_cookie = user_create(
+			'DupeMail' . rand(),
+			'password',
+			$t_email_with_case_variation
+		);
+		$t_user_id = user_get_id_by_cookie( $t_cookie );
+
+		$this->assertNotFalse(
+			user_get_id_by_email( self::TEST_EMAIL ),
+			"User found when multiple accounts with same email exist"
+		);
+		user_delete( $t_user_id );
+
+		// Expected failures
+		$this->assertFalse(
+			user_get_id_by_email( rand() . self::TEST_EMAIL ),
+			"Non-existing email not found"
+			 );
+
+		// Same test but with exception
+		$this->expectException( ClientException::class );
+		user_get_id_by_email( rand() . self::TEST_EMAIL, true );
+	}
+
+}