Merge branch 'master-2.24'

mantisbt · May 2, 2020 · 49fc1ce · 49fc1ce
2 parents 9267633 + 0d5a739
commit 49fc1ce
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 36 deletions.
diff --git a/api/rest/restcore/users_rest.php b/api/rest/restcore/users_rest.php
@@ -107,7 +107,7 @@ function rest_user_reset_password( \Slim\Http\Request $p_request, \Slim\Http\Res
 	);
 
 	$t_command = new UserResetPasswordCommand( $t_data );
-	$t_command->execute();
+	$t_result = $t_command->execute();
 
 	return $p_response->withStatus( HTTP_STATUS_NO_CONTENT );
 }
diff --git a/core/commands/UserResetPasswordCommand.php b/core/commands/UserResetPasswordCommand.php
@@ -33,6 +33,12 @@
  * }
  */
 class UserResetPasswordCommand extends Command {
+	/**
+	 * Constants for execute() method's return value.
+	 */
+	const RESULT_RESET = 'reset';
+	const RESULT_UNLOCK = 'unlock';
+
 	/**
 	 * @var integer The id of the user to delete.
 	 */
@@ -49,30 +55,41 @@ function __construct( array $p_data ) {
 
 	/**
 	 * Validate the data.
+	 * @throws ClientException
 	 */
 	function validate() {
-		$this->user_id_reset = (int)$this->query( 'id', null );
-		if( $this->user_id_reset <= 0 || !user_exists( $this->user_id_reset ) ) {
-			throw new ClientException( 'Invalid user id', ERROR_INVALID_FIELD_VALUE, array( 'id' ) );
-		}
-
-		# Ensure user has access level to delete users
+		# Ensure user has the required access level to reset passwords
 		if( !access_has_global_level( config_get_global( 'manage_user_threshold' ) ) ) {
 			throw new ClientException( 'Access denied to reset user password', ERROR_ACCESS_DENIED );
 		}
 
+		$this->user_id_reset = (int)$this->query( 'id', null );
+
+		# Make sure the account exists
 		$t_user = user_get_row( $this->user_id_reset );
-		if( $t_user === false ) { // cannot be
+		if( $t_user === false ) {
 			throw new ClientException( 'Invalid user id', ERROR_INVALID_FIELD_VALUE, array( 'id' ) );
 		}
 
-		# Ensure that the account to be reset is of equal or lower access to the
-		# current user.
+		# Mantis can't reset protected accounts' passwords, but if the
+		# account is locked, we allow the operation as Unlock
+		if( auth_can_set_password( $this->user_id_reset )
+			&& user_is_protected( $this->user_id_reset )
+			&& user_is_login_request_allowed( $this->user_id_reset )
+		) {
+			throw new ClientException(
+				'Password reset not allowed for protected accounts',
+				ERROR_PROTECTED_ACCOUNT
+			);
+		}
+
+		# Ensure that the account to be reset is of equal or lower access than
+		# the current user.
 		if( !access_has_global_level( $t_user['access_level'] ) ) {
 			throw new ClientException( 'Access denied to reset user password with higher access level', ERROR_ACCESS_DENIED );
 		}
 
-		# Check that we are not reseting the last administrator account
+		# Check that we are not resetting the last administrator account
 		$t_admin_threshold = config_get_global( 'admin_site_threshold' );
 		if( user_is_administrator( $this->user_id_reset ) &&
 			user_count_level( $t_admin_threshold, /* enabled */ true ) <= 1 ) {
@@ -86,17 +103,19 @@ function validate() {
 	 * Process the command.
 	 *
 	 * @returns array Command response
+	 * @throws ClientException
 	 */
 	protected function process() {
-		# If the password can be changed, we reset it, otherwise we unlock
-		# the account (i.e. reset failed login count)
-		$t_reset = auth_can_set_password( $this->user_id_reset );
-		if( $t_reset ) {
-			$t_result = user_reset_password( $this->user_id_reset );
-		} else {
-			$t_result = user_reset_failed_login_count_to_zero( $this->user_id_reset );
+		# If the password can be changed, reset it
+		if( auth_can_set_password( $this->user_id_reset )
+			&& user_reset_password( $this->user_id_reset )
+		) {
+			return array( 'action' => self::RESULT_RESET );
 		}
-		return array();
+
+		# Password can't be changed, unlock the account
+		# the account (i.e. reset failed login count)
+		user_reset_failed_login_count_to_zero( $this->user_id_reset );
+		return array( 'action' =>  self::RESULT_UNLOCK );
 	}
 }
-
diff --git a/core/user_api.php b/core/user_api.php
@@ -1725,6 +1725,7 @@ function user_set_name( $p_user_id, $p_username ) {
  * @param integer $p_user_id    A valid user identifier.
  * @param boolean $p_send_email Whether to send confirmation email.
  * @return boolean
+ * @throws ClientException
  */
 function user_reset_password( $p_user_id, $p_send_email = true ) {
 	$t_protected = user_get_field( $p_user_id, 'protected' );
@@ -1743,6 +1744,11 @@ function user_reset_password( $p_user_id, $p_send_email = true ) {
 		$t_email = user_get_field( $p_user_id, 'email' );
 		if( is_blank( $t_email ) ) {
 			trigger_error( ERROR_LOST_PASSWORD_NO_EMAIL_SPECIFIED, ERROR );
+			throw new ClientException(
+				sprintf( "User id '%d' does not have an e-mail address.", (int)$p_user_id ),
+				ERROR_LOST_PASSWORD_NO_EMAIL_SPECIFIED,
+				array( (int)$p_user_id )
+			);
 		}
 
 		# Create random password

diff --git a/manage_user_reset.php b/manage_user_reset.php
@@ -52,33 +52,32 @@
 );
 
 $t_command = new UserResetPasswordCommand( $t_data );
-$t_command->execute();
+# The case of trying to reset a protected account now causes the Command to
+# trigger an exception, so we do not need any special handling here.
+$t_result = $t_command->execute();
 
 $t_redirect_url = 'manage_user_page.php';
 
 form_security_purge( 'manage_user_reset' );
 
-layout_page_header( null, $t_result ? $t_redirect_url : null );
-
+layout_page_header( null, $t_redirect_url );
 layout_page_begin( 'manage_overview_page.php' );
 
-if( $t_reset ) {
-	if( false == $t_result ) {
-		# PROTECTED
-		html_operation_failure( $t_redirect_url, lang_get( 'account_reset_protected_msg' ) );
-	} else {
-		# SUCCESSFUL RESET
-		if( ( ON == config_get( 'send_reset_password' ) ) && ( ON == config_get( 'enable_email_notification' ) ) ) {
-			# send the new random password via email
+switch( $t_result['action'] ) {
+	case UserResetPasswordCommand::RESULT_RESET:
+		if(    ( ON == config_get( 'send_reset_password' ) )
+			&& ( ON == config_get( 'enable_email_notification' ) )
+		) {
+			# Password reset confirmation sent by email
 			html_operation_successful( $t_redirect_url, lang_get( 'account_reset_msg' ) );
 		} else {
-			# email notification disabled, then set the password to blank
+			# Email notification disabled, password set to blank
 			html_operation_successful( $t_redirect_url, lang_get( 'account_reset_msg2' ) );
 		}
-	}
-} else {
-	# UNLOCK
-	html_operation_successful( $t_redirect_url, lang_get( 'account_unlock_msg' ) );
+		break;
+	case UserResetPasswordCommand::RESULT_UNLOCK:
+		html_operation_successful( $t_redirect_url, lang_get( 'account_unlock_msg' ) );
+		break;
 }
 
 layout_page_end();