View Issue Details

IDProjectCategoryView StatusLast Update
0011801mantisbtbugtrackerpublic2014-09-23 18:05
Reportermattmccutchen Assigned Todregad  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.0 
Target Version1.2.9Fixed in Version1.2.9 
Summary0011801: Permission error when clearing a version field of multiple bugs
Description

I tried to clear the "Fixed in Version" field of multiple bugs at once using the controls at the bottom of the "View Issues" page. However, Mantis said "You did not have appropriate permissions to perform that action", even though I am a site administrator. The problem does not occur when I set "Fixed in Version" to a nonempty value.

I believe the problem is that when Mantis checks that the requested version is valid in the project of the bug being changed, version_get_id will return false for the empty string. A special case for the empty string needs to be introduced here (and similarly for the target version):

http://git.mantisbt.org/?p=mantisbt.git;a=blob;f=bug_actiongroup.php;hb=a47272749c917f191915a192a73d64fa460ca29a#l255

Also, there should be a better error message when the version is not valid for a bug.

Tagspatch
Attached Files
bug-actiongroup-error-handling-cleanup.patch (8,474 bytes)   
bug_actiongroup.php: Clean up error handling.

- Always check the requester's access first.

- Associate each check directly with an error message, instead of
  performing all checks and going back to see which one failed.

- New error messages distinct from the general "permission denied"
  message when assigning an issue to a disallowed handler or setting a
  version that does not exist in the project.

Includes string changes for English only.

diff --git a/bug_actiongroup.php b/bug_actiongroup.php
index 4e70bb1..a378242 100644
--- a/bug_actiongroup.php
+++ b/bug_actiongroup.php
@@ -107,19 +107,17 @@ foreach( $f_bug_arr as $t_bug_id ) {
 
 	case 'CLOSE':
 		$t_closed = config_get( 'bug_closed_status_threshold' );
-		if ( access_can_close_bug( $t_bug_id ) &&
-				( $t_status < $t_closed ) &&
+		if ( access_can_close_bug( $t_bug_id ) ) {
+			if ( ( $t_status < $t_closed ) &&
 				bug_check_workflow( $t_status, $t_closed ) ) {
-
-			/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $f_bug_id, $t_bug_data, $f_bugnote_text ) ); */
-			bug_close( $t_bug_id, $f_bug_notetext, $f_bug_noteprivate );
-			helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
-		} else {
-			if ( !access_can_close_bug( $t_bug_id ) ) {
-				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
+				/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $f_bug_id, $t_bug_data, $f_bugnote_text ) ); */
+				bug_close( $t_bug_id, $f_bug_notetext, $f_bug_noteprivate );
+				helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
 			} else {
 				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_status' );
 			}
+		} else {
+			$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
 		}
 		break;
 
@@ -163,38 +161,38 @@ foreach( $f_bug_arr as $t_bug_id ) {
 		# check that new handler has rights to handle the issue, and
 		#  that current user has rights to assign the issue
 		$t_threshold = access_get_status_threshold( $t_assign_status, bug_get_field( $t_bug_id, 'project_id' ) );
-		if ( access_has_bug_level( $t_threshold , $t_bug_id, $f_assign ) &&
-			 access_has_bug_level( config_get( 'update_bug_assign_threshold', config_get( 'update_bug_threshold' ) ), $t_bug_id ) &&
-				bug_check_workflow($t_status, $t_assign_status )	) {
-			/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
-			bug_assign( $t_bug_id, $f_assign, $f_bug_notetext, $f_bug_noteprivate );
-			helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
-		} else {
-			if ( bug_check_workflow($t_status, $t_assign_status ) ) {
-				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
+		if ( access_has_bug_level( config_get( 'update_bug_assign_threshold', config_get( 'update_bug_threshold' ) ), $t_bug_id ) ) {
+			if ( access_has_bug_level( $t_threshold , $t_bug_id, $f_assign ) ) {
+				if ( bug_check_workflow($t_status, $t_assign_status ) ) {
+					/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
+					bug_assign( $t_bug_id, $f_assign, $f_bug_notetext, $f_bug_noteprivate );
+					helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
+				} else {
+					$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_status' );
+				}
 			} else {
-				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_status' );
+				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_handler' );
 			}
+		} else {
+			$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
 		}
 		break;
 
 	case 'RESOLVE':
 		$t_resolved_status = config_get( 'bug_resolved_status_threshold' );
-		if ( access_has_bug_level( access_get_status_threshold( $t_resolved_status, bug_get_field( $t_bug_id, 'project_id' ) ), $t_bug_id ) &&
-					( $t_status < $t_resolved_status ) &&
-					bug_check_workflow($t_status, $t_resolved_status ) ) {
-			$f_resolution = gpc_get_int( 'resolution' );
-			$f_fixed_in_version = gpc_get_string( 'fixed_in_version', '' );
-			/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
-			bug_resolve( $t_bug_id, $f_resolution, $f_fixed_in_version, $f_bug_notetext, null, null, $f_bug_noteprivate );
-			helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
-		} else {
+		if ( access_has_bug_level( access_get_status_threshold( $t_resolved_status, bug_get_field( $t_bug_id, 'project_id' ) ), $t_bug_id ) ) {
 			if ( ( $t_status < $t_resolved_status ) &&
 					bug_check_workflow($t_status, $t_resolved_status ) ) {
-				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
+				$f_resolution = gpc_get_int( 'resolution' );
+				$f_fixed_in_version = gpc_get_string( 'fixed_in_version', '' );
+				/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
+				bug_resolve( $t_bug_id, $f_resolution, $f_fixed_in_version, $f_bug_notetext, null, null, $f_bug_noteprivate );
+				helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
 			} else {
 				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_status' );
 			}
+		} else {
+			$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
 		}
 		break;
 
@@ -249,18 +247,16 @@ foreach( $f_bug_arr as $t_bug_id ) {
 	case 'UP_FIXED_IN_VERSION':
 		$f_fixed_in_version = gpc_get_string( 'fixed_in_version' );
 		$t_project_id = bug_get_field( $t_bug_id, 'project_id' );
-		$t_success = false;
 
 		if ( access_has_bug_level( config_get( 'update_bug_threshold' ), $t_bug_id ) ) {
 			if ( version_get_id( $f_fixed_in_version, $t_project_id ) !== false ) {
 				/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
 				bug_set_field( $t_bug_id, 'fixed_in_version', $f_fixed_in_version );
 				helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
-				$t_success = true;
+			} else {
+				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_version' );
 			}
-		}
-
-		if ( !$t_success ) {
+		} else {
 			$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
 		}
 		break;
@@ -268,18 +264,16 @@ foreach( $f_bug_arr as $t_bug_id ) {
 	case 'UP_TARGET_VERSION':
 		$f_target_version = gpc_get_string( 'target_version' );
 		$t_project_id = bug_get_field( $t_bug_id, 'project_id' );
-		$t_success = false;
 
 		if ( access_has_bug_level( config_get( 'roadmap_update_threshold' ), $t_bug_id ) ) {
 			if ( version_get_id( $f_target_version, $t_project_id ) !== false ) {
 				/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
 				bug_set_field( $t_bug_id, 'target_version', $f_target_version );
 				helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
-				$t_success = true;
+			} else {
+				$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_version' );
 			}
-		}
-
-		if ( !$t_success ) {
+		} else {
 			$t_failed_ids[$t_bug_id] = lang_get( 'bug_actiongroup_access' );
 		}
 		break;
diff --git a/lang/strings_english.txt b/lang/strings_english.txt
index cced653..60e5055 100644
--- a/lang/strings_english.txt
+++ b/lang/strings_english.txt
@@ -139,8 +139,10 @@ $s_select_option = '(select)'; # comboboxes default to this to instruct user to
 
 # bug_actiongroup_page.php : mass treatment
 $s_bug_actiongroup_access = 'You did not have appropriate permissions to perform that action.';
-$s_bug_actiongroup_status = 'This issue cannot be changed to the requested status';
-$s_bug_actiongroup_category = 'This issue cannot be changed to the requested category';
+$s_bug_actiongroup_status = 'The requested status change is not allowed by the workflow.';
+$s_bug_actiongroup_handler = 'The requested handler is not allowed to handle this issue.';
+$s_bug_actiongroup_category = 'The requested category does not exist.';
+$s_bug_actiongroup_version = 'The requested version does not exist in this issue\'s project.';
 $s_close_bugs_conf_msg = 'Are you sure you wish to close these issues?';
 $s_delete_bugs_conf_msg = 'Are you sure you wish to delete these issues?';
 $s_move_bugs_conf_msg = 'Move issues to';
bug-actiongroup-allow-empty-version.patch (1,549 bytes)   
bug_actiongroup.php: Allow setting an empty version.

diff --git a/bug_actiongroup.php b/bug_actiongroup.php
index a378242..7c0ab32 100644
--- a/bug_actiongroup.php
+++ b/bug_actiongroup.php
@@ -249,7 +249,7 @@ foreach( $f_bug_arr as $t_bug_id ) {
 		$t_project_id = bug_get_field( $t_bug_id, 'project_id' );
 
 		if ( access_has_bug_level( config_get( 'update_bug_threshold' ), $t_bug_id ) ) {
-			if ( version_get_id( $f_fixed_in_version, $t_project_id ) !== false ) {
+			if ( $f_fixed_in_version === '' || version_get_id( $f_fixed_in_version, $t_project_id ) !== false ) {
 				/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
 				bug_set_field( $t_bug_id, 'fixed_in_version', $f_fixed_in_version );
 				helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );
@@ -266,7 +266,7 @@ foreach( $f_bug_arr as $t_bug_id ) {
 		$t_project_id = bug_get_field( $t_bug_id, 'project_id' );
 
 		if ( access_has_bug_level( config_get( 'roadmap_update_threshold' ), $t_bug_id ) ) {
-			if ( version_get_id( $f_target_version, $t_project_id ) !== false ) {
+			if ( $f_target_version === '' || version_get_id( $f_target_version, $t_project_id ) !== false ) {
 				/** @todo we need to issue a helper_call_custom_function( 'issue_update_validate', array( $t_bug_id, $t_bug_data, $f_bugnote_text ) ); */
 				bug_set_field( $t_bug_id, 'target_version', $f_target_version );
 				helper_call_custom_function( 'issue_update_notify', array( $t_bug_id ) );

Relationships

related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 
has duplicate 0012988 closedatrol Cannot unset the "Target Version" field on Resolved issues using the group action button from the "View Issues" page 
related to 0013661 closeddregad Improve messages when errors occur in bug group actions 

Activities

mattmccutchen

mattmccutchen

2010-04-16 04:44

reporter   ~0025136

The attached patches clean up the error handling and allow clearing the version fields.

Alain Frisch

Alain Frisch

2011-05-17 07:54

reporter   ~0028789

I've tested this patch and it seems to work for me.

dregad

dregad

2011-12-08 13:22

developer   ~0030519

Just experienced this myself. I'll review the patch later on, as time allows.

dregad

dregad

2011-12-10 18:13

developer   ~0030547

The original bug (ie. failure to clear the fixed/target version field in multi-bug updates) has been fixed.

For better traceability, the error handling and reporting improvement will be tracked in new issue 0013661.

grangeway

grangeway

2013-04-05 17:57

reporter   ~0036351

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

Related Changesets

MantisBT: master 970fa723

2011-12-10 09:46

dregad


Details Diff
Allow clearing fixed in/target version for multiple bugs

Prior to this, trying to set the "Fixed in version" or "Target version"
fields to blank for multiple bugs at once (from the "View Issues" page)
failed with error "You did not have appropriate permissions to perform
that action", even when executed as admin. Problem does not occur when
changing to a non-empty value.

Fixes 0011801
Affected Issues
0011801
mod - bug_actiongroup.php Diff File

MantisBT: master-1.2.x 6afa5f78

2011-12-10 09:46

dregad


Details Diff
Allow clearing fixed in/target version for multiple bugs

Prior to this, trying to set the "Fixed in version" or "Target version"
fields to blank for multiple bugs at once (from the "View Issues" page)
failed with error "You did not have appropriate permissions to perform
that action", even when executed as admin. Problem does not occur when
changing to a non-empty value.

Fixes 0011801
Affected Issues
0011801
mod - bug_actiongroup.php Diff File