Opened 5 months ago
Last modified 2 months ago
#57394 assigned defect (bug)
wp_insert_user allows the new user to have a username equal to an already registered email
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | 6.1.1 |
Component: | Login and Registration | Keywords: | revert has-patch changes-requested needs-testing needs-unit-tests |
Focuses: | Cc: |
Description
Scenario:
User A is an administrator, has username = 'admin' and email = 'admin@…';
User B registers (open to register wp install) with username = 'admin@…' and email = 'test@…';
In my case, i use both users, one to admin the website, and the other to simulate an customer user. Both users had same password.
When i tried to log in with administrator user by email, i end up logged in the as user B.
Then i changed User B password to not conflict, and it works as it should, if i set user A password it gets logged in as user A and if i use User B password it gets logged in as User B.
I don't think this is a security issue, but allowing an user to be registered with another users email as username could be annoying and confuse some website administrators for scams or something else.
To solve this issue, i just filtered username_exists filter and checked if the username was existing email.
public function __construct() {
add_filter("username_exists", array($this, "username_exists"));
}
public function username_exists($user_id, $username) {
if (email_exists($username)) {
$user = get_user_by("email", $username);
if ($user->exists()) {
return $user->ID;
}
}
return $user_id;
}
Attachments (6)
Change History (34)
@
5 months ago
Added check to determine if username supplied already exists as an email of an existing user.
This ticket was mentioned in PR #3809 on WordPress/wordpress-develop by JosephGabito.
5 months ago
#2
- Keywords has-patch added; needs-patch removed
---
Trac ticket: https://core.trac.wordpress.org/ticket/57394
@dunhakdis commented on PR #3809:
5 months ago
#3
@mukeshpanchal27 Thanks. I've applied your suggestions as commit. Cheers
#4
@
5 months ago
A possible privacy concern could be that this would allow a bad actor to try entering email addresses until it hits the new error message (assuming an open registration form). They would then know that that address is in use.
#5
@
5 months ago
I think that is also true for the default WordPress registration form with or without these changes.
@
4 months ago
Created a unit test that attempts to create a new user with admin email as a username.
#8
@
4 months ago
- Keywords has-unit-tests added; needs-unit-tests removed
@ajayver , Thanks for adding a unit-test.
This ticket was mentioned in PR #4094 on WordPress/wordpress-develop by @adamsilverstein.
4 months ago
#9
Trac ticket: https://core.trac.wordpress.org/ticket/57394
#10
@
4 months ago
- Keywords needs-unit-tests added; has-unit-tests removed
- Milestone changed from Awaiting Review to 6.2
- Owner set to adamsilverstein
- Status changed from new to assigned
This ticket was mentioned in PR #4095 on WordPress/wordpress-develop by @adamsilverstein.
4 months ago
#13
- Keywords has-unit-tests added; needs-unit-tests removed
Trac ticket: https://core.trac.wordpress.org/ticket/57394
This ticket was mentioned in Slack in #core by polevaultweb. View the logs.
3 months ago
This ticket was mentioned in PR #4257 on WordPress/wordpress-develop by polevaultweb.
3 months ago
#16
This patch improves the logic around the username checks added in WP 6.2, to make sure the check doesn't cause issues when updating a user.
Trac ticket: https://core.trac.wordpress.org/ticket/57967
polevaultweb commented on PR #4257:
3 months ago
#17
Thanks @costdev, applied and updated
#18
@
3 months ago
- Keywords revert added
- Resolution fixed deleted
- Status changed from closed to reopened
#19
@
3 months ago
- Owner changed from adamsilverstein to audrasjb
- Status changed from reopened to assigned
3 months ago
#20
Wondering if this PR should be repurposed or a new one opened to implement the logic discussed in https://github.com/WordPress/wordpress-develop/pull/4257#discussion_r1144203722 and the following comments.
#24
@
2 months ago
- Keywords dev-reviewed added; dev-feedback removed
[55584] is ready for backport to 6.2 branch.
#26
@
2 months ago
- Keywords changes-requested needs-testing needs-unit-tests added; has-unit-tests dev-reviewed removed
- Milestone changed from 6.2 to 6.3
The changesets have been reverted. Additional considerations are needed to address the issue raised #57967 and potentially other scenarios.
With 6.2 Dry Run and final release next week, it's too late in the cycle to continue working on this. Moving to 6.3.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
2 months ago
#28
@
2 months ago
Test Report
This report validates that the indicated patch addresses the issue.
Patch tested: https://core.trac.wordpress.org/ticket/57394
Environment
- OS: macOS 13.0.1
- Web Server: Nginx
- PHP: 7.4.24
- WordPress: 6.3-alpha-55505-src
- Browser: Chrome 109.0.5414.119 (Official Build) (x86_64)
- Theme: Twenty Twenty-Three
- Active Plugins: -
Actual Results
- ✅ Issue resolved with patch.
I was able to reproduce the same behaviour. I think it should throw a WP_Error if the supplied username already exists as an email of an existing user.
The same behaviour can be reproduced with wp-login.php?action=register form.
Added a patch.