Using the same activation key results in wrong "Invalid activation key" error

[r-a-y]

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.

[DJPaul]

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?).

[DJPaul]

See also #7914

[dsar]

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.

[Jean-David]

Not sure if my commit doesn't make any side-effect, I'd like some feedback on this, tanks.
Basically, in the request, there is always

$sql['where'][] = "active = 0";

Removing the first index is safe as it's declared earlier in the code as the first key :

$sql['where']   = array();
$sql['where'][] = "active = 0";

[imath]

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.

[dcavins]

In 13099:

Introduce $active parameter for BP_Signup::get().

Allow the BP_Signup::get() method to fetch

• activated signups only (active = 1)
• not-yet-activated signups only (active = 0) - the default case
• signups with any active status (active = false)

This change means that users following an activation key
for an already-activated account will get a more helpful
error message telling them the account is already activated.

Props Jean-David.

Fixes #7938.