#44672 closed defect (bug) (fixed)
REST-API: invalid email on lowercase/uppercase change
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.1 | Priority: | normal |
Severity: | normal | Version: | 4.9.7 |
Component: | REST API | Keywords: | has-patch has-unit-tests commit |
Focuses: | Cc: |
Description
When changing an email adresse from lower to uppercase (and vice versa) the error "rest_user_invalid_email" is thrown. This is wrong and it should work as in edit_user().
Examples:
[REST-API] change email from "UserName@domain.com" to "username@…" results in ERROR
[WP-admin] change email from "UserName@domain.com" to "username@…" results in SUCCESS
Both functions use different types of comparisons, while edit_user() is correct and REST-API wrong:
WP_REST_Users_Controller > update_item():
<?php if ( email_exists( $request['email'] ) && $request['email'] !== $user->user_email )
wp-admin/includes/user.php > edit_user()
<?php $user->user_email = sanitize_text_field( wp_unslash( $_POST['email'] ) ); [...] elseif ( ( $owner_id = email_exists($user->user_email) ) && ( !$update || ( $owner_id != $user->ID ) ) )
Attachments (3)
Change History (18)
#3
@
7 years ago
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
- Milestone changed from Awaiting Review to 5.0
In 44672.diff I modified the comparison that checks if a provided user email address should be allowed to change in a PUT request to transform the comparison check of current email address <> provided value to always check lowercase strings.
This feels a bit hacky, but will result in allowing a user to change:
Editor@… --> editor@… successfully matching WP-admin.
#5
@
6 years ago
- Milestone changed from 5.0 to 5.1
Bumping to 5.1 because this isn't critical for the 5.0 release.
This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.
6 years ago
#8
@
6 years ago
- Keywords commit removed
44672.2.diff is a different approach that is more aligned with the check performed in edit_user()
. email_exists()
is a case insensitive lookup.
I also added a test to ensure a user cannot change their email to one that is assigned to another user if the casing is changed.
This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.
6 years ago
#11
@
6 years ago
44672.2.diff looks good to me. For precision, can we make sure test_update_item_existing_email_case_not_own
has an explicit assertion for the 'rest_user_invalid_email' error code too?
#12
@
6 years ago
Looks pretty good, although the assignment in this conditional
if ( ( $owner_id = email_exists( $request['email'] ) ) && $owner_id !== $id ) {
does not thrill me. I'd like to propose adding a comment to make it clear we're borrowing verbatim, or find a way to clarify the logic flow.
#13
@
6 years ago
- Keywords commit added
44672.3.diff moves the assignment outside of the conditional and adds an explicit assertion for the rest_user_invalid_email
code to test_update_item_existing_email_case_not_own
.
Can we simply
unslash
and thensanitize
the value like