Fix sensitive data exposure vulnerability in /pm/v2/users API endpoints#573
Conversation
WalkthroughIntroduces two new permission classes (Create_Users and List_Users) for granular access control in user-related operations. Updates routes to enforce Create_Users permission for user creation and List_Users permission for user listing, search, and project viewing endpoints, replacing generic Authentic permission. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
routes/user.php (1)
24-26: Consider adding route-level permission for consistency.The comment indicates the controller checks
manage_options, but relying solely on controller-level authorization is less robust. For consistency with the other routes in this file, consider creating a dedicated permission class or using an existing one that enforcesmanage_optionsat the route level.Defense-in-depth: route-level permissions are checked before the controller is even instantiated, providing an earlier security gate.
-// User meta update - already checks manage_options in controller -$wedevs_pm_router->post( 'save_users_map_name', 'WeDevs/PM/User/Controllers/User_Controller@save_users_map_name' ) - ->permission(['WeDevs\PM\Core\Permissions\Authentic']); +// User meta update - require manage_options capability +$wedevs_pm_router->post( 'save_users_map_name', 'WeDevs/PM/User/Controllers/User_Controller@save_users_map_name' ) + ->permission(['WeDevs\PM\Core\Permissions\Manage_Options']); // Or create this class
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/Permissions/Create_Users.phpcore/Permissions/List_Users.phproutes/user.php
🧰 Additional context used
🧬 Code graph analysis (1)
core/Permissions/Create_Users.php (3)
core/Permissions/Abstract_Permission.php (1)
Abstract_Permission(8-32)core/Permissions/List_Users.php (1)
check(13-39)views/assets/src/helpers/i18n/i18n.js (1)
__(45-47)
🔇 Additional comments (4)
core/Permissions/Create_Users.php (1)
13-34: Permission logic is correct and appropriately restrictive.The implementation correctly enforces that only users with the
create_userscapability (typically administrators) can create users. The asymmetry withList_Users(which also allows PM managers) appears intentional—user creation is a more privileged operation than listing.The 401/403 distinction for unauthenticated vs. unauthorized access follows REST API best practices.
Please confirm this asymmetry is intentional: PM managers can list users but cannot create them. If PM managers should also be able to create users, add the
wedevs_pm_user_can_access()check as done inList_Users.php.core/Permissions/List_Users.php (1)
13-39: Well-structured permission check with appropriate dual-path authorization.The implementation correctly provides two authorization paths:
- WordPress core
list_userscapability (typically admins)- PM-specific manager capability via
wedevs_pm_user_can_access()This ensures both WordPress administrators and PM managers can access user listing endpoints while blocking lower-privileged users like subscribers—directly addressing the IDOR vulnerability.
routes/user.php (2)
7-15: Security fix correctly applied to all user listing endpoints.All GET endpoints for user data are now properly protected with
List_Userspermission, closing the IDOR vulnerability.One consideration:
GET users/{id}now requireslist_userscapability. Verify whether users should be able to view their own profile data. If self-access is a valid use case, theList_Userspermission class may need an additional check:// Allow users to view their own data if ( $this->request->get_param('id') == $user_id ) { return true; }
17-19: User creation endpoint properly secured.The POST users endpoint is now protected with
Create_Userspermission, preventing unauthorized user creation.
iftakharul-islam
left a comment
There was a problem hiding this comment.
All everything looks good.
Thanks Ariful @arifulhoque7
Security: Fix broken access control in User API endpoints (IDOR)
Close 276
The /wp-json/pm/v2/users endpoints were accessible to any authenticated
user (including subscribers), exposing sensitive user data such as emails,
usernames, and external account identifiers (GitHub/Bitbucket).
Changes:
Affected endpoints:
Severity: Medium (CVSS 6.5)
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.