Permissions: normalize for_site() usage for _metabox() and save() (#73)

This change fixes a bug causing User Roles to be incorrectly set when saving the "Permissions" tab on multisite installations. It also shifts the order of switch_to_blog() and current_user_can() calls, to ensure that Roles are only saved when the current user can "promote_users" on the current site for the user being edited (never self!) It also also includes a little bit of surrounding code clean-up while I'm here. Props chrisvanpatten. Fixes #70.
stuttter · Aug 18, 2021 · d2b2275 · d2b2275
1 parent 87095e6
commit d2b2275
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 27 deletions.
diff --git a/wp-user-profiles/includes/languages/wp-user-profiles.pot b/wp-user-profiles/includes/languages/wp-user-profiles.pot
@@ -5,7 +5,7 @@ msgstr ""
 "Project-Id-Version: WP User Profiles 2.6.1\n"
 "Report-Msgid-Bugs-To: "
 "https://github.com/stuttter/wp-user-profiles/issues/new\n"
-"POT-Creation-Date: 2021-08-14 06:48:30+00:00\n"
+"POT-Creation-Date: 2021-08-18 18:32:42+00:00\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=utf-8\n"
 "Content-Transfer-Encoding: 8bit\n"
@@ -315,7 +315,7 @@ msgstr ""
 msgid "This user has no role on any sites"
 msgstr ""
 
-#: wp-user-profiles/includes/metaboxes/permissions-roles.php:100
+#: wp-user-profiles/includes/metaboxes/permissions-roles.php:108
 msgid "&mdash; No role for this site &mdash;"
 msgstr ""
 
@@ -475,19 +475,19 @@ msgstr ""
 msgid "Fields without explicit sections"
 msgstr ""
 
-#: wp-user-profiles/includes/sections/permissions.php:107
+#: wp-user-profiles/includes/sections/permissions.php:130
 msgid "This is where role & capability settings can be found."
 msgstr ""
 
-#: wp-user-profiles/includes/sections/permissions.php:108
+#: wp-user-profiles/includes/sections/permissions.php:131
 msgid "Your role determines what you are able to do"
 msgstr ""
 
-#: wp-user-profiles/includes/sections/permissions.php:109
+#: wp-user-profiles/includes/sections/permissions.php:132
 msgid "In some cases, you may have more than one role"
 msgstr ""
 
-#: wp-user-profiles/includes/sections/permissions.php:110
+#: wp-user-profiles/includes/sections/permissions.php:133
 msgid "Some capabilities may be uniquely granted"
 msgstr ""
 

diff --git a/wp-user-profiles/includes/metaboxes/permissions-roles.php b/wp-user-profiles/includes/metaboxes/permissions-roles.php
@@ -69,42 +69,50 @@ function wp_user_profiles_roles_metabox( $user = null ) {
 				// Switch to this site
 				if ( is_multisite() ) {
 
-					// Skip if user cannot manage
+					// Switch site early
+					switch_to_blog( $site_id );
+
+					// User cannot be promoted on this site by current user
 					if ( ( $current_site_id !== $site_id ) && ! current_user_can( 'promote_user', $user->ID ) ) {
+
+						// Switch site back
+						restore_current_blog();
+
+						// Skip to next site
 						continue;
 					}
 
-					switch_to_blog( $site_id );
-
-					// Reinitialize the User roles & caps for this Site ID
+					// Reinitialize the user roles & caps for this site ID
 					$user->for_site( $site_id );
-				} ?>
+				}
+
+				// Compare user role against currently editable roles
+				$editable_roles = get_editable_roles();
+				$user_roles     = array_intersect( array_values( $user->roles ), array_keys( $editable_roles ) );
+				$user_role      = reset( $user_roles );
+
+				?>
 
 				<tr class="user-role-wrap">
 					<th scope="row">
-						<label for="role[<?php echo $site_id; ?>]">
+						<label for="role[<?php echo (int) $site_id; ?>]">
 							<?php echo esc_html( $site->blogname ); ?><br>
-							<span class="description"><?php echo $site->siteurl; ?></span>
+							<span class="description"><?php echo esc_html( $site->siteurl ); ?></span>
 						</label>
 					</th>
-					<td><select name="role[<?php echo $site_id; ?>]" id="role[<?php echo $site_id; ?>]" <?php disabled( $is_self, true ); ?>>
-							<?php
-
-							// Compare user role against currently editable roles
-							$user_roles = array_intersect( array_values( $user->roles ), array_keys( get_editable_roles() ) );
-							$user_role  = reset( $user_roles );
+					<td><select name="role[<?php echo (int) $site_id; ?>]" id="role[<?php echo (int) $site_id; ?>]" <?php disabled( $is_self, true ); ?>><?php
 
 							// Print the full list of roles
-							wp_dropdown_roles( $user_role ); ?>
+							wp_dropdown_roles( $user_role );
 
-							<option value="" <?php selected( empty( $user_role ) ); ?>><?php esc_html_e( '&mdash; No role for this site &mdash;', 'wp-user-profiles' ); ?></option>
+							?><option value="" <?php selected( empty( $user_role ) ); ?>><?php esc_html_e( '&mdash; No role for this site &mdash;', 'wp-user-profiles' ); ?></option>
 						</select>
 					</td>
 				</tr>
 
 				<?php
 
-				// Switch back to this site
+				// Switch back to the current site
 				if ( is_multisite() ) {
 					restore_current_blog();
 

diff --git a/wp-user-profiles/includes/sections/permissions.php b/wp-user-profiles/includes/sections/permissions.php
@@ -65,27 +65,50 @@ public function save( $user = null ) {
 		// Role changes
 		if ( isset( $_POST['role'] ) && is_array( $_POST['role'] ) && current_user_can( $this->cap, $user->ID ) ) {
 
+			// Stash the current Site ID for later reuse
+			$current_site_id = get_current_blog_id();
+
 			// Loop through new roles
-			foreach ( $_POST['role'] as $blog_id => $new_role ) {
+			foreach ( $_POST['role'] as $site_id => $new_role ) {
 
 				// Switch to the blog
 				if ( is_multisite() ) {
-					switch_to_blog( $blog_id );
+
+					// Switch site early
+					switch_to_blog( $site_id );
+
+					// User cannot be promoted on this site by current user
+					if ( ( $current_site_id !== $site_id ) && ! current_user_can( 'promote_user', $user->ID ) ) {
+
+						// Switch site back
+						restore_current_blog();
+
+						// Skip to next site
+						continue;
+					}
+
+					// Reinitialize the user roles & caps for this site ID
+					$user->for_site( $site_id );
 				}
 
-				// Only allow switching to editable role for site
+				// Get roles for this site
 				$editable_roles = get_editable_roles();
+
+				// Only allow editable roles for switched site
 				if ( ! empty( $new_role ) && ! empty( $editable_roles[ $new_role ] ) ) {
 					$user->set_role( $new_role );
 
-				// Or remove all caps if no role for site
+				// ...or remove all caps if no role for site
 				} elseif ( empty( $new_role ) ) {
 					$user->remove_all_caps();
 				}
 
-				// Switch back
+				// Switch back to the current site
 				if ( is_multisite() ) {
 					restore_current_blog();
+
+					// Reset the user's role & capabilities
+					$user->for_site( $current_site_id );
 				}
 			}
 		}