Activation link - change from "?key=" to action variable

[r-a-y]

Referenced in a support forum thread:
​http://jrfom.com/2014/08/11/buddypress-and-lighttpd/

This might be causing a lot of the activation email issues that we've been having.

Let's switch from using this:
example.com/activate/?key=XXX

to:
example.com/activate/XXX/

or:
example.com/activate/key/xxx/

While keeping the older $_GET method for backward compatiblity.

[boonebgorges]

Sounds fine to me.

See also ​https://core.trac.wordpress.org/ticket/25350

[DJPaul]

I've just seen ​http://bloggingatmidnight.wordpress.com/2014/08/26/topic-new-user-invalid-activation-key-the-solution-%c2%b7-buddypress-org/ which is interesting to look at.

[r-a-y]

I think ?key hits spam / malware email filters a certain way. Both the WordPress ticket and the bloggingatmidnight post kind of confirm this.

We just need some testers once this ticket has a patch :)

[r-a-y]

01.patch changes the activation link to example.com/activate/XXX/ and also supports activating with older links (example.com/activate/?key=XXX).

Needs testing for those that have had activation email problems in the past.

[DJPaul]

Patch looks OK, we need to figure out how to create users in the unit tests that aren't automatically activated so we can test this.

[DJPaul]

Upon consideration, this would be more of an integration test, especially now we have BP_Signups class, so I'm not going to worry about that.

[djpaul]

In 9109:

Signups: move the activation link's key from the query param into the URL proper.

Some email hosts strip the key from the activation link, or mangle it, or prevent it being easily copyable inside their web interfaces. Some web servers/configurations, can also be commonly configured to strip query parameters from the majority of requests, so hopefully this change will alleviate these problems.

Compatibility is maintained for the older query param-style activation links.

Fixes #5831, props r-a-y.

[johnjamesjacoby]

This is certainly going to break backwards compatibility for anyone checking the $_GET on their own.

Can we post something on BP Devel? I think we should encapsulate the key getting logic in a function to direct people to; if our new approach doesn't fix the issue (or breaks again years from now) the function gives us a way to change the innards without breaking changes again.

[vimes1984]

I've just experienced a host having a spam filter installed that follows the external validation link in the outbound email, activating the users accounts always automatically rendering the activation link useless...
​https://buddypress.trac.wordpress.org/ticket/6049
Am happy to help how i can :D

[r-a-y]

In 9203:

Signups: Do not bail if no key is found on the activation screen.

r9109 broke how the activation page is rendered if no key is passed in
the URL. For example, if you go to example.com/activate/ instead of
example.com/activate/?key=XXX or example.com/activate/XXX/.

Anti-props r-a-y.

See #5831.

[r-a-y]

This is certainly going to break backwards compatibility for anyone checking the $_GET on their own.

If plugins were doing checks for the key on their own, wouldn't the plugin need to remove how BP does its activation process with:

remove_action( 'bp_screens', 'bp_core_screen_activation' );

I think we should encapsulate the key getting logic in a function to direct people to

Sure, I think that's fair.

[vimes1984]

can i help with anything? I've tried the IRC but it seems pretty dead...

[r-a-y]

vimes1984 - Your comments should be left in #6049. This is a slightly, different issue.

I've tried the IRC but it seems pretty dead...

We now use Slack instead of IRC. Check out this post for more info on how to get on Slack:
​https://bpdevel.wordpress.com/2014/10/28/were-going-to-move-our/

[r-a-y]

jjj - Have you read comment:11?

I don't see a way how a dev checks the old $_GET['key'] and adds their own implementation without getting rid of the existing activation screen.

Here's the old code for reference:
​https://buddypress.trac.wordpress.org/browser/tags/2.1.1/src/bp-members/bp-members-screens.php#L214

If there are problems with this implementation, can you let us know?

I'm going to leave this ticket open during beta.

[johnjamesjacoby]

In 9400:

Review r9109 and r9203, and clean up bp_core_screen_activation() to improve readability and separate a few procedural tasks. Fixes #5831.

[DJPaul]

We missed several instances of these; I'm going to address in 2.5 with other email changes.