From: <gi...@ma...> - 2010-03-31 13:49:34
|
The branch, master-1.2.x has been updated via 67f43bde8b6e170fc64f4daa2f37f9dd76ed890b (commit) from f845ed542e27bc760086157e3f5559d8c5fd1b0b (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 67f43bde8b6e170fc64f4daa2f37f9dd76ed890b Author: David Hicks <hic...@op...> Date: Thu Apr 1 00:17:53 2010 +1100 Fix #11530: Support multiple access levels above manage_user_threshold Traditionally manage_user_threshold was thought of as being an absolute global threshold which would allow any user the ability to modify any other user account. Thus manage_user_threshold effectively had to be the same as admin_site_threshold because users with manage_user_threshold could just modify accounts to escalate their permissions to the maximum level. This patch prevents users from modifying accounts which have an access level greater than their own. It also prevents users from creating accounts with with access levels greater than their own. Thus it is now possible to use manage_user_threshold as a separate permission level to admin_site_threshold. Users with an access level between manage_user_threshold <= user access level < admin_site_threshold can no longer escalate their permissions or modify the accounts of other users with a higher access level. ----------------------------------------------------------------------- Summary of changes: account_prefs_reset.php | 3 ++ account_prefs_update.php | 3 ++ core/project_api.php | 49 ++++++++++++++++++++++++++++++++++-------- manage_proj_edit_page.php | 2 +- manage_proj_user_copy.php | 2 +- manage_proj_user_remove.php | 6 ++++- manage_user_create.php | 4 +++ manage_user_create_page.php | 2 +- manage_user_delete.php | 4 +++ manage_user_edit_page.php | 11 +++++---- manage_user_page.php | 8 +++++++ manage_user_proj_add.php | 5 +++- manage_user_proj_delete.php | 5 ++++ manage_user_prune.php | 7 ++++- manage_user_reset.php | 6 +++++ manage_user_update.php | 20 +++++++++++++---- 16 files changed, 110 insertions(+), 27 deletions(-) ----------------------------------------------------------------------- commit 67f43bde8b6e170fc64f4daa2f37f9dd76ed890b Author: David Hicks <hic...@op...> Date: Thu Apr 1 00:17:53 2010 +1100 Fix #11530: Support multiple access levels above manage_user_threshold Traditionally manage_user_threshold was thought of as being an absolute global threshold which would allow any user the ability to modify any other user account. Thus manage_user_threshold effectively had to be the same as admin_site_threshold because users with manage_user_threshold could just modify accounts to escalate their permissions to the maximum level. This patch prevents users from modifying accounts which have an access level greater than their own. It also prevents users from creating accounts with with access levels greater than their own. Thus it is now possible to use manage_user_threshold as a separate permission level to admin_site_threshold. Users with an access level between manage_user_threshold <= user access level < admin_site_threshold can no longer escalate their permissions or modify the accounts of other users with a higher access level. diff --git a/account_prefs_reset.php b/account_prefs_reset.php index 3fcac32..51fd2d7 100644 --- a/account_prefs_reset.php +++ b/account_prefs_reset.php @@ -53,11 +53,14 @@ user_ensure_exists( $f_user_id ); + $t_user = user_get_row( $f_user_id ); + # This page is currently called from the manage_* namespace and thus we # have to allow authorised users to update the accounts of other users. # TODO: split this functionality into manage_user_prefs_reset.php if ( auth_get_current_user_id() != $f_user_id ) { access_ensure_global_level( config_get( 'manage_user_threshold' ) ); + access_ensure_global_level( $t_user['access_level'] ); } else { # Protected users should not be able to update the preferences of their # user account. The anonymous user is always considered a protected diff --git a/account_prefs_update.php b/account_prefs_update.php index b1fa873..4ac5245 100644 --- a/account_prefs_update.php +++ b/account_prefs_update.php @@ -37,11 +37,14 @@ user_ensure_exists( $f_user_id ); + $t_user = user_get_row( $f_user_id ); + # This page is currently called from the manage_* namespace and thus we # have to allow authorised users to update the accounts of other users. # TODO: split this functionality into manage_user_prefs_update.php if ( auth_get_current_user_id() != $f_user_id ) { access_ensure_global_level( config_get( 'manage_user_threshold' ) ); + access_ensure_global_level( $t_user['access_level'] ); } else { # Protected users should not be able to update the preferences of their # user account. The anonymous user is always considered a protected diff --git a/core/project_api.php b/core/project_api.php index b38d5c3..c3fb4c0 100644 --- a/core/project_api.php +++ b/core/project_api.php @@ -690,25 +690,47 @@ function project_remove_user( $p_project_id, $p_user_id ) { return true; } -# delete all users from the project user list for a given project -# this is useful when deleting or closing a project -function project_remove_all_users( $p_project_id ) { +/** + * Delete all users from the project user list for a given project. This is + * useful when deleting or closing a project. The $p_access_level_limit + * parameter can be used to only remove users from a project if their access + * level is below or equal to the limit. + * @param int Project ID + * @param int Access level limit (null = no limit) + * @return true + */ +function project_remove_all_users( $p_project_id, $p_access_level_limit = null ) { $t_project_user_list_table = db_get_table( 'mantis_project_user_list_table' ); $c_project_id = db_prepare_int( $p_project_id ); $query = "DELETE FROM $t_project_user_list_table - WHERE project_id=" . db_param(); + WHERE project_id=" . db_param(); - db_query_bound( $query, Array( $c_project_id ) ); + if ( $p_access_level_limit !== null ) { + $c_access_level_limit = db_prepare_int( $p_access_level_limit ); + $query .= " AND access_level <= " . db_param(); + db_query_bound( $query, Array( $c_project_id, $c_access_level_limit ) ); + } else { + db_query_bound( $query, Array( $c_project_id ) ); + } # db_query errors on failure so: return true; } -# Copy all users and their permissions from the source project to the -# destination project -function project_copy_users( $p_destination_id, $p_source_id ) { +/** + * Copy all users and their permissions from the source project to the + * destination project. The $p_access_level_limit parameter can be used to + * limit the access level for users as they're copied to the destination + * project (the highest access level they'll receieve in the destination + * project will be equal to $p_access_level_limit). + * @param int Destination project ID + * @param int Source project ID + * @param int Access level limit (null = no limit) + * @return null + */ +function project_copy_users( $p_destination_id, $p_source_id, $p_access_level_limit = null ) { # Copy all users from current project over to another project $t_rows = project_get_local_user_rows( $p_source_id ); @@ -716,12 +738,19 @@ function project_copy_users( $p_destination_id, $p_source_id ) { for ( $i = 0; $i < $t_count; $i++ ) { $t_row = $t_rows[$i]; + if ( $p_access_level_limit !== null && + $t_row['access_level'] > $p_access_level_limit ) { + $t_destination_access_level = $p_access_level_limit; + } else { + $t_destination_access_level = $t_row['access_level']; + } + # if there is no duplicate then add a new entry # otherwise just update the access level for the existing entry if ( project_includes_user( $p_destination_id, $t_row['user_id'] ) ) { - project_update_user_access( $p_destination_id, $t_row['user_id'], $t_row['access_level'] ); + project_update_user_access( $p_destination_id, $t_row['user_id'], $t_destination_access_level ); } else { - project_add_user( $p_destination_id, $t_row['user_id'], $t_row['access_level'] ); + project_add_user( $p_destination_id, $t_row['user_id'], $t_destination_access_level ); } } } diff --git a/manage_proj_edit_page.php b/manage_proj_edit_page.php index 8e60af6..c4fc0b7 100644 --- a/manage_proj_edit_page.php +++ b/manage_proj_edit_page.php @@ -790,7 +790,7 @@ if ( $t_can_manage_users ) { <?php # You need global or project-specific permissions to remove users # from this project - if ( $t_can_manage_users ) { + if ( $t_can_manage_users && access_has_project_level( $t_user['access_level'], $f_project_id ) ) { if ( project_includes_user( $f_project_id, $t_user['id'] ) ) { print_button( "manage_proj_user_remove.php?project_id=$f_project_id&user_id=" . $t_user['id'], lang_get( 'remove_link' ) ); $t_removable_users_exist = true; diff --git a/manage_proj_user_copy.php b/manage_proj_user_copy.php index c00fb46..c80cda9 100644 --- a/manage_proj_user_copy.php +++ b/manage_proj_user_copy.php @@ -51,7 +51,7 @@ access_ensure_project_level( config_get( 'manage_project_threshold' ), $t_dst_project_id ); access_ensure_project_level( config_get( 'project_user_threshold' ), $t_dst_project_id ); - project_copy_users( $t_dst_project_id, $t_src_project_id ); + project_copy_users( $t_dst_project_id, $t_src_project_id. access_get_project_level( $t_dst_project_id ) ); form_security_purge( 'manage_proj_user_copy' ); diff --git a/manage_proj_user_remove.php b/manage_proj_user_remove.php index a48c0f7..c8a7fe9 100644 --- a/manage_proj_user_remove.php +++ b/manage_proj_user_remove.php @@ -41,9 +41,13 @@ # Confirm with the user helper_ensure_confirmed( lang_get( 'remove_all_users_sure_msg' ), lang_get( 'remove_all_users_button' ) ); - project_remove_all_users( $f_project_id ); + project_remove_all_users( $f_project_id, access_get_project_level( $f_project_id ) ); } else { + # Don't allow removal of users from the project who have a higher access level than the current user + access_ensure_project_level( access_get_project_level( $f_project_id, $f_user_id ), $f_project_id ); + $t_user = user_get_row( $f_user_id ); + # Confirm with the user helper_ensure_confirmed( lang_get( 'remove_user_sure_msg' ) . '<br/>' . lang_get( 'username' ) . ': ' . $t_user['username'], diff --git a/manage_user_create.php b/manage_user_create.php index 5b7cc3c..b41b833 100644 --- a/manage_user_create.php +++ b/manage_user_create.php @@ -77,6 +77,10 @@ } } + # Don't allow the creation of accounts with access levels higher than that of + # the user creating the account. + access_ensure_global_level( $f_access_level ); + # Need to send the user creation mail in the tracker language, not in the creating admin's language # Park the current language name until the user has been created lang_push( config_get( 'default_language' ) ); diff --git a/manage_user_create_page.php b/manage_user_create_page.php index 55f5147..911c217 100644 --- a/manage_user_create_page.php +++ b/manage_user_create_page.php @@ -107,7 +107,7 @@ </td> <td> <select name="access_level"> - <?php print_enum_string_option_list( 'access_levels', config_get( 'default_new_account_access_level' ) ) ?> + <?php print_project_access_levels_option_list( config_get( 'default_new_account_access_level' ) ) ?> </select> </td> </tr> diff --git a/manage_user_delete.php b/manage_user_delete.php index 6bf33f4..3270574 100644 --- a/manage_user_delete.php +++ b/manage_user_delete.php @@ -34,6 +34,10 @@ $t_user = user_get_row( $f_user_id ); + # Ensure that the account to be deleted is of equal or lower access to the + # current user. + access_ensure_global_level( $t_user['access_level'] ); + # check that we are not deleting the last administrator account $t_admin_threshold = config_get_global( 'admin_site_threshold' ); if ( user_is_administrator( $f_user_id ) && diff --git a/manage_user_edit_page.php b/manage_user_edit_page.php index f5310aa..6e92111 100644 --- a/manage_user_edit_page.php +++ b/manage_user_edit_page.php @@ -44,6 +44,10 @@ $t_user = user_get_row( $t_user_id ); + # Ensure that the account to be updated is of equal or lower access to the + # current user. + access_ensure_global_level( $t_user['access_level'] ); + $t_ldap = ( LDAP == config_get( 'login_method' ) ); html_page_top(); @@ -118,7 +122,7 @@ </td> <td> <select name="access_level"> - <?php print_enum_string_option_list( 'access_levels', $t_user['access_level'] ) ?> + <?php print_project_access_levels_option_list( config_get( 'default_new_account_access_level' ) ) ?> </select> </td> </tr> @@ -236,10 +240,7 @@ </td> <td> <select name="access_level"> - <?php - # No administrator choice - print_project_access_levels_option_list( config_get( 'default_new_account_access_level' ) ) - ?> + <?php print_project_access_levels_option_list( config_get( 'default_new_account_access_level' ) ) ?> </select> </td> </tr> diff --git a/manage_user_page.php b/manage_user_page.php index 380b239..5b74909 100644 --- a/manage_user_page.php +++ b/manage_user_page.php @@ -303,7 +303,15 @@ ?> <tr <?php echo helper_alternate_class( $i ) ?>> <td> + <?php + if ( access_has_global_level( $u_access_level ) ) { + ?> <a href="manage_user_edit_page.php?user_id=<?php echo $u_id ?>"><?php echo string_display_line( $u_username ) ?></a> + <? + } else { + echo string_display_line( $u_username ); + } + ?> </td> <td><?php echo string_display_line( $u_realname ) ?></td> <td><?php print_email_link( $u_email, $u_email ) ?></td> diff --git a/manage_user_proj_add.php b/manage_user_proj_add.php index 4ac960e..062d764 100644 --- a/manage_user_proj_add.php +++ b/manage_user_proj_add.php @@ -34,8 +34,11 @@ $f_project_id = gpc_get_int_array( 'project_id', array() ); $t_manage_user_threshold = config_get( 'manage_user_threshold' ); + user_ensure_exists( $f_user_id ); + foreach ( $f_project_id as $t_proj_id ) { - if ( access_has_project_level( $t_manage_user_threshold, $t_proj_id ) ) { + if ( access_has_project_level( $t_manage_user_threshold, $t_proj_id ) && + access_has_project_level( $f_access_level, $t_proj_id ) ) { project_add_user( $t_proj_id, $f_user_id, $f_access_level ); } } diff --git a/manage_user_proj_delete.php b/manage_user_proj_delete.php index 8c12912..4b487b8 100644 --- a/manage_user_proj_delete.php +++ b/manage_user_proj_delete.php @@ -32,7 +32,12 @@ $f_project_id = gpc_get_int( 'project_id' ); $f_user_id = gpc_get_int( 'user_id' ); + user_ensure_exists( $f_user_id ); + + $t_user = user_get_row( $f_user_id ); + access_ensure_project_level( config_get( 'project_user_threshold' ), $f_project_id ); + access_ensure_project_level( $t_user['access_level'], $f_project_id ); $t_project_name = project_get_name( $f_project_id ); diff --git a/manage_user_prune.php b/manage_user_prune.php index d2b8585..0c2d88e 100644 --- a/manage_user_prune.php +++ b/manage_user_prune.php @@ -36,7 +36,7 @@ # Delete the users who have never logged in and are older than 1 week $days_old = (int)7 * SECONDS_PER_DAY; - $query = "SELECT id + $query = "SELECT id, access_level FROM $t_user_table WHERE ( login_count = 0 ) AND ( date_created = last_visit ) AND " . db_helper_compare_days( 0, "date_created", "> $days_old" ); $result = db_query_bound($query, Array( db_now() ) ); @@ -54,7 +54,10 @@ for ($i=0; $i < $count; $i++) { $row = db_fetch_array( $result ); - user_delete($row['id']); + # Don't prune accounts with a higher global access level than the current user + if ( access_has_global_level( $row['access_level'] ) ) { + user_delete($row['id']); + } } form_security_purge( 'manage_user_prune' ); diff --git a/manage_user_reset.php b/manage_user_reset.php index 3910efe..98f011e 100644 --- a/manage_user_reset.php +++ b/manage_user_reset.php @@ -34,6 +34,12 @@ user_ensure_exists( $f_user_id ); + $t_user = user_get_row( $f_user_id ); + + # Ensure that the account to be reset is of equal or lower access to the + # current user. + access_ensure_global_level( $t_user['access_level'] ); + $t_result = user_reset_password( $f_user_id ); $t_redirect_url = 'manage_user_page.php'; diff --git a/manage_user_update.php b/manage_user_update.php index 7228601..cd8da96 100644 --- a/manage_user_update.php +++ b/manage_user_update.php @@ -48,17 +48,23 @@ user_ensure_exists( $f_user_id ); + $t_user = user_get_row( $f_user_id ); + $f_email = trim( $f_email ); $f_username = trim( $f_username ); - $t_old_username = user_get_field( $f_user_id, 'username' ); + $t_old_username = $t_user['username']; if ( $f_send_email_notification ) { - $t_old_realname = user_get_field( $f_user_id, 'realname' ); - $t_old_email = user_get_email( $f_user_id ); - $t_old_access_level = user_get_field( $f_user_id, 'access_level' ); + $t_old_realname = $t_user['realname']; + $t_old_email = $t_user['email']; + $t_old_access_level = $t_user['access_level']; } + # Ensure that the account to be updated is of equal or lower access to the + # current user. + access_ensure_global_level( $t_user['access_level'] ); + # check that the username is unique if ( 0 != strcasecmp( $t_old_username, $f_username ) && false == user_is_name_unique( $f_username ) ) { @@ -86,7 +92,11 @@ $t_user_table = db_get_table( 'mantis_user_table' ); - $t_old_protected = user_get_field( $f_user_id, 'protected' ); + $t_old_protected = $t_user['protected']; + + # Ensure that users aren't escalating privileges of accounts beyond their + # own global access level. + access_ensure_global_level( $f_access_level ); # check that we are not downgrading the last administrator $t_admin_threshold = config_get_global( 'admin_site_threshold' ); ----------------------------------------------------------------------- -- Mantis Bug Tracker |