Skip to content

fix: allow admins to edit other users' settings#4934

Open
GaryJones wants to merge 159 commits intobuddyboss:releasefrom
GaryJones:fix/bp-core-can-edit-settings
Open

fix: allow admins to edit other users' settings#4934
GaryJones wants to merge 159 commits intobuddyboss:releasefrom
GaryJones:fix/bp-core-can-edit-settings

Conversation

@GaryJones
Copy link
Copy Markdown

Description

bp_core_can_edit_settings() has a logic error that prevents administrators from editing other users' settings on the front end. The elseif ( bp_is_user() && ! bp_is_my_profile() ) branch catches all "other user" cases, setting $status = false before the bp_moderate / edit_users capability check is ever reached.

This branch is also redundant — $status is already initialised to false, so non-admin users viewing other profiles already get false without needing an explicit branch.

Removing the branch restores the logic to match upstream BuddyPress.

Fixes #4933

Steps to reproduce (before fix)

  1. Log in as an administrator
  2. Navigate to /members/other-user/profile/edit/
  3. Observe redirect to the member's profile with "You do not have access to this page"

Expected behaviour (after fix)

Administrators with bp_moderate or edit_users capability can edit other members' profile fields on the front end, as intended by the capability check that was previously unreachable.

mehedi-bb and others added 23 commits July 1, 2025 19:56
* release: (57 commits)
  grunt release
  grunt
  Update: Grunt string-replace
  version bump
  grunt
  Exclude vendor for checktextdomain
  cleanup repo
  shows action count as unlimited
  grunt fix
  updated URL to be production one
  implement copy clipboard to the licence key
  Fix the fields to show
  text changes
  Created popup for free licence licence creation
  Implement cache for the fetch license details
  added a free plugin id
  update condition to show the bb header in admin
  updated code to check the add on available or not
  restructure licence information
  added lciense info
  ...

# Conflicts:
#	src/composer.lock
* release:
  grunt release
  version bump
  grunt
  Added change log
  PROD-9249 - fix the issue to check the issue exists and must be array befor array_merge
  PROD-9249 - run migration for admin only
  PROD-9249 - updated the mothership load priority
  composer update
* release:
  grunt release
  version bump and added change log
  grunt
  Fix deprecation: Use AbstractPluginConnection::class instead of CONNECTION_PLUGIN_SERVICE_ID
  break the loop when ilcense activated
  improve error validation on migration
  update the code to run once on update specific version
  removed unused things
  fix the connector
  Called migration on plugin update only
  Deprecated:  BuddyBossPlatform\GroundLevel\Mothership\Service::CONNECTION_PLUGIN_SERVICE_ID()
  fix the error display issue
* release:
  grunt release
  grunt
  version bump and added change log
  PROD-9260 - restrict few items to be admin specific
  PROD-9260 - Fix the issue about to work the pro feature on frontend
* release:
  grunt release
  grunt
  version bump and added change log
  [PROD-9270] Composer update to fix the issue with the theme update issue
The elseif branch `bp_is_user() && ! bp_is_my_profile()` in
bp_core_can_edit_settings() catches all cases where a user views
another user's profile, setting status to false before the
bp_moderate / edit_users capability check is reached. This prevents
administrators from editing other members' settings on the front end.

The branch is also redundant since $status is already initialised to
false. Removing it restores the logic to match upstream BuddyPress,
where the capability check is correctly evaluated.

Fixes buddyboss#4933
@chetansatasiya chetansatasiya changed the base branch from master to release April 15, 2026 13:21
@chetansatasiya
Copy link
Copy Markdown
Contributor

@claude Review the PR

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Claude finished @chetansatasiya's task in 1m 29s —— View job


PR Review: fix: allow admins to edit other users' settings

  • Gather context and understand the request
  • Read changed file and surrounding code
  • Analyze the logic fix
  • Post review feedback

Summary

This is a clean, correct fix. The removed branch introduced a subtle logic bug that silently short-circuited the if/elseif chain, making the admin capability checks unreachable.

Logic Analysis

Before (buggy):

$status = false;

if ( bp_is_my_profile() ) {
    $status = true;
} elseif ( bp_is_user() && ! bp_is_my_profile() ) {
    $status = false;           // ← catches ALL "other user" cases here
} elseif ( is_super_admin( bp_displayed_user_id() ) && ! is_super_admin() ) {
    $status = false;           // ← UNREACHABLE when on another user's profile
} elseif ( bp_current_user_can( 'bp_moderate' ) || current_user_can( 'edit_users' ) ) {
    $status = true;            // ← UNREACHABLE when on another user's profile
}

When an admin visits /members/other-user/settings/:

  • bp_is_my_profile() → false (different user)
  • bp_is_user() && ! bp_is_my_profile()true — chain stops, returns false
  • The bp_moderate / edit_users capability check is never evaluated

After (fixed):

$status = false;

if ( bp_is_my_profile() ) {
    $status = true;
} elseif ( is_super_admin( bp_displayed_user_id() ) && ! is_super_admin() ) {
    $status = false;           // Prevent non-super-admins from editing super admins
} elseif ( bp_current_user_can( 'bp_moderate' ) || current_user_can( 'edit_users' ) ) {
    $status = true;            // ← Now reachable for admins on other users' profiles
}

Correctness of All Cases

Scenario Before After
User views own settings true true
Regular user views another user's settings false false ✓ (default)
Admin (bp_moderate/edit_users) views another user's settings false true
Admin views super admin settings false (wrong branch) false ✓ (super admin guard)
Super admin views another super admin's settings false (wrong branch) true ✓ (capability check)

Redundancy of the Removed Branch

The removed branch was also logically redundant. $status is initialized to false on line 1506. Non-admin users viewing another profile already receive false without any explicit elseif — the removed branch was dead code in that sense too.

Scope of Impact

bp_core_can_edit_settings() is used in 13 call sites across components (bp-settings, bp-xprofile, bp-messages, bp-notifications, bp-friends, bp-groups, bp-invites, templates, REST API, LearnDash integration). All these will now correctly allow admins to edit other users' settings.

Verdict

Approve. The fix is minimal, correct, and matches the description. The PR author correctly identifies both the bug cause (premature elseif catch) and the redundancy. No issues found.

One minor observation — not a blocker: the is_super_admin( bp_displayed_user_id() ) && ! is_super_admin() branch can also now only be reached when bp_is_my_profile() is false (i.e., an admin viewing someone else's page), which is the correct context for that guard. The logic reads more naturally after the removal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bp_core_can_edit_settings() prevents admins from editing other users' settings

6 participants