View Issue Details

WikiRelated ChangesetsJump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0011782mantisbtbugtrackerpublic2010-04-11 16:072014-12-22 08:23
Reportermattmccutchen Assigned Todregad  
PriorityhighSeveritymajorReproducibilityhave not tried
Status closedResolutionfixed 
Product Version1.2.0 
Target Version1.2.12Fixed in Version1.2.12 
Summary0011782: allow_reporter_reopen should only apply to users with reporter access
Description

If a user reports an issue and then his/her access level is downgraded to "viewer", the user would no longer be able to add bugnotes to the issue, but the allow_reporter_reopen option would still allow him/her to reopen it. I imagine most sites would not want that. I propose that allow_reporter_reopen should only apply to users that currently have at least reporter access.

Additional Information

The SuperTux bug tracker (http://supertux.lethargik.org/bugs/) previously had a Guest account with reporter access. The access was reduced to viewer due to abuse, but we are still seeing drive-by reopenings of existing bugs that have Guest as the reporter due to allow_reporter_reopen. See http://supertux.lethargik.org/bugs/view.php?id=634 .

Tagspatch
Attached Files
allow-reporter-for-reporters-only.patch (4,817 bytes)    
Give allow_reporter_* privileges only to users who still have reporter access to the bug.

diff --git a/bug_change_status_page.php b/bug_change_status_page.php
index 77cb099..c4efd5a 100644
--- a/bug_change_status_page.php
+++ b/bug_change_status_page.php
@@ -56,6 +56,7 @@
 
 	if ( !( ( access_has_bug_level( access_get_status_threshold( $f_new_status, bug_get_field( $f_bug_id, 'project_id' ) ), $f_bug_id ) ) ||
 				( ( bug_get_field( $f_bug_id, 'reporter_id' ) == $t_current_user_id ) &&
+					( access_has_bug_level( REPORTER, $f_bug_id ) ) &&
 						( ( ON == config_get( 'allow_reporter_reopen' ) ) ||
 								( ON == config_get( 'allow_reporter_close' ) ) ) ) ||
 				( ( ON == $f_reopen_flag ) && ( access_has_bug_level( config_get( 'reopen_bug_threshold' ), $f_bug_id ) ) )
diff --git a/bug_update_advanced_page.php b/bug_update_advanced_page.php
index 79fe390..fa8e26d 100644
--- a/bug_update_advanced_page.php
+++ b/bug_update_advanced_page.php
@@ -357,6 +357,7 @@ if ( $tpl_show_status || $tpl_show_resolution ) {
 		echo '<td bgcolor="', get_status_color( $tpl_bug->status ), '">';
 		print_status_option_list( 'status', $tpl_bug->status,
 							( $tpl_bug->reporter_id == auth_get_current_user_id() &&
+								access_has_bug_level( REPORTER, $f_bug_id ) &&
 									( ON == config_get( 'allow_reporter_close' ) ) ), $tpl_bug->project_id );
 		echo '</td>';
 	} else {
diff --git a/core/access_api.php b/core/access_api.php
index 7aa8db8..ba8fe2c 100644
--- a/core/access_api.php
+++ b/core/access_api.php
@@ -505,7 +505,8 @@ function access_has_bugnote_level( $p_access_level, $p_bugnote_id, $p_user_id =
 	}
 
 	# If allow_reporter_close is enabled, then reporters can always close their own bugs
-	if( ON == config_get( 'allow_reporter_close' ) && bug_is_user_reporter( $p_bug_id, $p_user_id ) ) {
+	if( ON == config_get( 'allow_reporter_close' ) && bug_is_user_reporter( $p_bug_id, $p_user_id ) &&
+		access_has_bug_level( REPORTER, $p_bug_id, $p_user_id ) ) {
 		return true;
 	}
 
@@ -542,7 +543,8 @@ function access_has_bugnote_level( $p_access_level, $p_bugnote_id, $p_user_id =
 	}
 
 	# If allow_reporter_reopen is enabled, then reporters can always reopen their own bugs
-	if( ON == config_get( 'allow_reporter_reopen' ) && bug_is_user_reporter( $p_bug_id, $p_user_id ) ) {
+	if( ON == config_get( 'allow_reporter_reopen' ) && bug_is_user_reporter( $p_bug_id, $p_user_id ) &&
+		access_has_bug_level( REPORTER, $p_bug_id, $p_user_id ) ) {
 		return true;
 	}
 
diff --git a/core/file_api.php b/core/file_api.php
index 4729d4d..cc62014 100644
--- a/core/file_api.php
+++ b/core/file_api.php
@@ -788,7 +788,8 @@ function file_allow_bug_upload( $p_bug_id = null, $p_user_id = null ) {
 	# *** If we ever wanted to have a per-project setting enabling file
 	#     uploads, we'd want to check it here before exempting the reporter
 
-	if( $t_reporter && ( ON == config_get( 'allow_reporter_upload' ) ) ) {
+	if( $t_reporter && ( ON == config_get( 'allow_reporter_upload' ) ) &&
+		( null === $p_bug_id || access_has_bug_level( REPORTER, $p_bug_id, $p_user_id ) ) ) {
 		return true;
 	}
 
diff --git a/core/html_api.php b/core/html_api.php
index 05a8587..023267f 100644
--- a/core/html_api.php
+++ b/core/html_api.php
@@ -1347,7 +1347,7 @@ function html_button_bug_change_status( $p_bug_id ) {
 	$t_bug_current_state = bug_get_field( $p_bug_id, 'status' );
 	$t_current_access = access_get_project_level( $t_bug_project_id );
 
-	$t_enum_list = get_status_option_list( $t_current_access, $t_bug_current_state, false, ( bug_get_field( $p_bug_id, 'reporter_id' ) == auth_get_current_user_id() && ( ON == config_get( 'allow_reporter_close' ) ) ), $t_bug_project_id );
+	$t_enum_list = get_status_option_list( $t_current_access, $t_bug_current_state, false, ( bug_get_field( $p_bug_id, 'reporter_id' ) == auth_get_current_user_id() && ( ON == config_get( 'allow_reporter_close' ) ) && access_has_bug_level( REPORTER, $p_bug_id ) ), $t_bug_project_id );
 
 	if( count( $t_enum_list ) > 0 ) {
 
@@ -1514,7 +1514,8 @@ function html_button_bug_reopen( $p_bug_id ) {
 	$t_reopen_status = config_get( 'bug_reopen_status', null, null, $t_project );
 
 	if( access_has_bug_level( config_get( 'reopen_bug_threshold', null, null, $t_project ), $p_bug_id ) ||
-			(( bug_get_field( $p_bug_id, 'reporter_id' ) == auth_get_current_user_id() ) && ( ON == config_get( 'allow_reporter_reopen', null, null, $t_project ) ) ) ) {
+			(( bug_get_field( $p_bug_id, 'reporter_id' ) == auth_get_current_user_id() ) && ( ON == config_get( 'allow_reporter_reopen', null, null, $t_project ) )
+				&& access_has_bug_level( REPORTER, $p_bug_id ) ) ) {
 		html_button( 'bug_change_status_page.php', lang_get( 'reopen_bug_button' ), array( 'id' => $p_bug_id, 'new_status' => $t_reopen_status, 'reopen_flag' => ON ) );
 	}
 }
allow-reporter-for-reporters-only.patch (4,817 bytes)   

Relationships

related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 

Activities

dhx

dhx

2010-09-18 00:51

reporter   ~0026765

Good catch.

My only comment is that we shouldn't be checking against the REPORTER constant as this is hard-coded and may not exist in someones custom access level enum.

We should instead check against config_get( 'report_bug_threshold' )
jamesbond2011

jamesbond2011

2012-07-18 03:16

reporter   ~0032344

Hi,

I am having same issue. Using MantisBT 1.2.11 (just upgraded from 1.2.4 in last week)
Previously it was working fine. But now, Reopen button only shown if user had reported the issue.
Instead it should allow to reopen the issue to all users who has 'reporter' access level currently.
dregad

dregad

2012-08-30 06:23

developer   ~0032724

But now, Reopen button only shown if user had reported the issue.

This is the intended behavior, as documented [1] for $g_allow_reporter_reopen

"If set, the bug reporter is allowed to reopen their own bugs once resolved"

[1] http://mantisbt.org/docs/master-1.2.x/en/administration_guide/admin.config.status.html
dregad

dregad

2012-11-04 12:57

developer   ~0034224

This should be fixed as part of the branch https://github.com/dregad/mantisbt/tree/manage-config-workflow (see 0014496).

Testing and feedback welcome.
dregad

dregad

2012-11-05 10:53

developer   ~0034241

It appears that in 1.3.x this issue is already resolved when the code in bug_update.php was refactored (see attached changeset)
grangeway

grangeway

2013-04-05 17:56

reporter   ~0036137

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

Related Changesets

MantisBT: master 035a1302

2010-06-22 20:44

dhx


Details Diff		 Refactor bug_update to fix multiple bugs

This is a large change to bug_update.php which refactors the code to
make it clearer and easier to understand. More importantly this
refactoring fixes a number of bugs including the prior ability for
reporters (with allow_reporter_reopen enabled) to make any modifications
to the bug in question even when they didn't have permission to make
those changes.

This refactoring brings about a structural change to the bug update
process. Previously the update checked a field (or number of fields) and
then committed changes to the database before moving on to the next
field. Hence it was possible for some of the requested changes to be
committed to the database before a validation error kicked in,
preventing the remainder of updates from being committed.

The new structure of bug_update prevents partial commits from occurring
as all validation and access checks are done prior to ANY data being
committed to the database. If all the validation checks pass then the
user can be safe in knowing that all changes should be committed to the
database. If any of the validation checks fail, none of the changes have
been committed.

One remaining problem with this approach is the race condition whereby
the administrator updates access checks between the validation of a
field and the attempted committal of a field to the database. As access
checks are performed internally within bug_api (and elsewhere), these
could actually fail during committal (and after the validation steps in
bug_update) if the access levels have changed in the meantime. This is a
problem with requires rewriting much of the MantisBT codebase - all for
a problem that is unlikely to occur and which is of low severity.

Email notifications also need to be sorted out correctly some time in
the future as it is hard to determine what the expected course of action
should be. This update tries sending an email in this order: resolved,
closed, reopened, handler added, status changed, bug updated. Only one
email is sent so if the handler and status of an issue are updated at
once and a user has refused to receive handler notifications, they won't
get any email. This is because we currently give higher priority to
notifying users of the addition of a handler to an issue rather than a
change of status.

Issue 0012097: Refactor bug_update.php
Fixes 0009828: Reopen issue access check is wrong
Fixes 0010226: No email on 'update->assign'
Fixes 0011396: difference between closed and resolved
Fixes 0011804: allow_reporter_reopen lets reporter make any update		 Affected Issues
0009828, 0010226, 0011396, 0011782, 0011804, 0012097
mod - bug_update.php Diff File

MantisBT: master-1.2.x a7f9febe

2012-08-30 06:52

dregad


Details Diff		 Restrict $g_allow_reporter_* to users with reporter access

Prior to this, if a user reported an issue and then their access level
was downgraded to "viewer", they would no longer be able to add bugnotes
to the issue. However, config option $g_allow_reporter_reopen would
still let them reopen it, and likewise for $g_allow_reporter_upload to
attach files and $g_allow_reporter_close.

Fixes 0011782		 Affected Issues
0011782
mod - bug_update.php Diff File
mod - core/access_api.php Diff File
mod - core/bug_group_action_api.php Diff File
mod - core/file_api.php Diff File
mod - core/html_api.php Diff File