#46744 Prevent the default role from being set to a privileged role when user registration is open - #12977
#46744 Prevent the default role from being set to a privileged role when user registration is open#12977johnbillion wants to merge 5 commits into
Conversation
…r registration is open.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
audrasjb
left a comment
audrasjb
left a comment
There was a problem hiding this comment.
The logic looks good to me.
I'm quite fine with the filter_default_role function name as long as it is not used at all (at least no occurrence exists in the wpdirectory.net database yet), but we definitely need to add a docblock before the function, event if it basically just show the same content used below in the hook.
There was a problem hiding this comment.
Pull request overview
This PR hardens WordPress user registration by preventing the default_role option from effectively resolving to a privileged role when user registration is enabled, while also providing a new filter (default_role_excluded_roles) to customize which roles are disallowed.
Changes:
- Adds an
option_default_rolefilter to override privileged default roles tosubscriberwhen registration is open. - Introduces the
default_role_excluded_rolesfilter (defaulting toadministratorandeditor) and wires it into the General Settings role dropdown behavior. - Adds PHPUnit coverage for both direct filtering behavior and end-to-end user creation role assignment.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/phpunit/tests/functions/filterDefaultRole.php | Adds unit/integration tests covering filtering behavior and user creation role assignment. |
| src/wp-includes/functions.php | Introduces the filter_default_role() option filter callback and the default_role_excluded_roles filter. |
| src/wp-includes/default-filters.php | Hooks the new default-role filter callback to option_default_role. |
| src/wp-admin/options-general.php | Uses default_role_excluded_roles to seed the excluded roles used in the default role dropdown logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * @param string $default_role The default role for new user registrations. | ||
| * @return string The filtered default role for new user registrations. | ||
| */ | ||
| function filter_default_role( $default_role ) { |
audrasjb
left a comment
audrasjb
left a comment
There was a problem hiding this comment.
This looks good to me 👍
| * @return string The filtered default role for new user registrations. | ||
| */ | ||
| function filter_default_role( $default_role ) { | ||
| static $filtering = false; |
There was a problem hiding this comment.
Why Is there a static variable in use here? It feels unnecessary
This change prevents the default role from containing a dangerous value when user registration is open. It does this via filtering the
default_roleoption value, meaning a privileged value in the option in the database gets overridden.The Editor and Administrator roles are excluded by default (when user registration is open). The
default_role_excluded_rolesfilter is introduced to facilitate adding to or removing from the list of excluded roles.On a Multisite installation, the
users_can_register_signup_filter()filter is applied tooption_users_can_register. This PR doesn't need to handle that specifically, but tests have been added to ensure it's covered.Trac ticket: Core-46744
Use of AI Tools
Changes written manually, tests written by Opus 5.