Skip to content

Commit

Permalink
Fix reporting as a different user
Browse files Browse the repository at this point in the history
- Check that the other user has report issue note access.
- Attribute the note to the other user rather than the logged in user.
  • Loading branch information
vboctor committed Jan 10, 2018
1 parent 459f09f commit 2623cf3
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
22 changes: 14 additions & 8 deletions core/commands/IssueNoteAddCommand.php
Expand Up @@ -92,6 +92,11 @@ class IssueNoteAddCommand extends Command {
*/
private $text = '';

/**
* The reporter id for the note.
*/
private $reporterId = 0;

/**
* Constructor
*
Expand Down Expand Up @@ -132,10 +137,6 @@ function validate() {
array( $t_issue_id ) );
}

if( !access_has_bug_level( config_get( 'add_bugnote_threshold' ), $t_issue_id ) ) {
throw new ClientException( 'access denied', ERROR_ACCESS_DENIED );
}

$this->parseViewState();

if( $this->private ) {
Expand Down Expand Up @@ -182,19 +183,24 @@ function validate() {
# Parse reporter id or default it.
$t_reporter = $this->payload( 'reporter' );
if( $t_reporter !== null ) {
$t_reporter_id = user_get_id_by_user_info( $t_reporter );
$this->reporterId = user_get_id_by_user_info(
$t_reporter, /* throw_if_id_doesnt_exist */ true );
} else {
$t_reporter_id = $this->user_id;
$this->reporterId = $this->user_id;
}

if( $t_reporter_id != $this->user_id ) {
if( $this->reporterId != $this->user_id ) {
# Make sure that active user has access level required to specify a different reporter.
# This feature is only available in the API and not Web UI.
$t_specify_reporter_access_level = config_get( 'webservice_specify_reporter_on_add_access_level_threshold' );
if( !access_has_bug_level( $t_specify_reporter_access_level, $t_issue_id ) ) {
throw new ClientException( 'Access denied to override reporter', ERROR_ACCESS_DENIED );
}
}

if( !access_has_bug_level( config_get( 'add_bugnote_threshold' ), $t_issue_id, $this->reporterId ) ) {
throw new ClientException( "Reporter can't add notes", ERROR_ACCESS_DENIED );
}
}

/**
Expand Down Expand Up @@ -227,7 +233,7 @@ protected function process() {
$this->private,
BUGNOTE,
/* attr */ '',
/* user_id */ null,
/* user_id */ $this->reporterId,
/* send_email */ false );
if( !$t_note_id ) {
throw new ClientException( "Unable to add note", ERROR_GENERIC );
Expand Down
13 changes: 8 additions & 5 deletions core/user_api.php
Expand Up @@ -880,18 +880,21 @@ function user_get_id_by_realname( $p_realname, $p_throw = false ) {

/**
* Get a user id given an array that may have id, name, real_name, email, or name_or_realname.
* If user id is specified, it will returned with validating that it exists.
* This is to allow for calling APIs to decide whether existence is needed or not,
* for example when updating an issue that already had a deleted user as the reporter,
* it is OK to keep such reference as long as it doesn't change.
*
* @param array $p_user The user info.
* @param boolean $p_throw_if_id_not_found If id specified and doesn't exist, then throw.
* @return user id
* @throws ClientException
*/
function user_get_id_by_user_info( array $p_user ) {
function user_get_id_by_user_info( array $p_user, $p_throw_if_id_not_found = false ) {
if( isset( $p_user['id'] ) && (int)$p_user['id'] != 0 ) {
$t_user_id = $p_user['id'];
if( $p_throw_if_id_not_found && !user_exists( $t_user_id ) ) {
throw new ClientException(
sprintf( "User with id '%d' doesn't exist", $t_user_id ),
ERROR_USER_BY_ID_NOT_FOUND,
array( $t_user_id ) );
}
} else if( isset( $p_user['name'] ) && !is_blank( $p_user['name'] ) ) {
$t_user_id = user_get_id_by_name( $p_user['name'], /* throw */ true );
} else if( isset( $p_user['email'] ) && !is_blank( $p_user['email'] ) ) {
Expand Down

0 comments on commit 2623cf3

Please sign in to comment.