Opened 6 years ago
Closed 3 years ago
#7938 closed defect (bug) (fixed)
Using the same activation key results in wrong "Invalid activation key" error
Reported by: | r-a-y | Owned by: | dcavins |
---|---|---|---|
Milestone: | Awaiting Contributions | Priority: | normal |
Severity: | normal | Version: | |
Component: | Members | Keywords: | needs-unit-tests |
Cc: |
Description
Reported here:
https://buddypress.org/support/topic/invalid-activation-key-10/
Original report follows:
So I’ve been plagued by this from users who either try and use their activation email again, or some that double click on the link. They all end up on the classic ‘Invalid activation key’ page.
Looking at the buddypress code I think there is a bug (feature?) that means that once an activation code is used once, it will never be able to be used again. This is despite there being code further on in this process to check this scenario and return an error of ‘The user is already active’. This code NEVER RUNS, because of the way the key is looked up in the database.
once an account is activated (active=1), the key can never be looked up again. Hence the error my users were getting.
Forum thread also has code that attempts to address the issue.
Sounds like a valid report, but needs verification and testing.
Attachments (1)
Change History (8)
#4
@
5 years ago
The original report is not about conflicting keys, it's about users trying to activate keys after they had already been activated. At the moment, that will result in an "Invalid activation key" message for the user.
The code in bp-members-function.php, function bp_core_activate_signup()
if ( $signup->active ) {
if ( empty( $signup->domain ) ) {
return new WP_Error( 'already_active', __( 'The user is already active.', 'buddypress' ), $signup );
} else {
return new WP_Error( 'already_active', __( 'The site is already active.', 'buddypress' ), $signup );
}
}
will never run, as BP_Signup::get will never fetch non-activated keys from signups table, as it has this code in it:
$sql['where'] = array();
$sql['where'][] = "active = 0";
I'm not sure if just replacing
$sql['where'][] = "active = 0";
with
$sql['where'][] = "1 = 1";
would affect anything else, but it would fix this issue.
Having looked back on the changes in these two files, I'm not seeing changes to this part of the code in at least the last 5-6 years, which makes me wonder if anything else might be depending on it?
The issue with "Invalid activation key" error is easily reproduceable by just double clicking the Activate button for any new users.
#5
@
5 years ago
Not sure if my commit doesn't make any side-effect, I'd like some feedback on this, tanks.
#6
@
5 years ago
- Keywords needs-unit-tests added; needs-testing removed
Hi @Jean-David
Thanks a lot for your contribution, I guess it should fix the issue. It would be awesome if you could make sure it doesn't introduce side effects improving all our unit tests starting by test_get_with
into this file:
trunk/tests/phpunit/testcases/members/class-bp-signup.php
The idea is to make sure the active
field is 0
for those existing tests and to add a new test like test_get_with_activation_key_already_active
to check we get 1
for the active field value when the signup has been activated.
Assuming these codes are somewhat randomly generated in our end (MD5, I'd guess), it's amusing to me that we find reports with the key conflicting. I'm not sure where we store it, but if a user is activated, I can't see any reason why we need to keep that key around (we could delete it?).