Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 21 months ago

#6049 closed enhancement (fixed)

Do not activate user accounts automatically with one click

Reported by: vimes1984 Owned by: boonebgorges
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Members Keywords: has-patch needs-testing
Cc:

Description (last modified by r-a-y)

Just had to fix this on a shared hosting account @ https://www.a2hosting.com, they run a spam filter called https://www.barracuda.com/products/spamfirewall
what this does it follow external links in any outgoing emails sent from the server in question. So when buddypress sends out it's activation link like so : http://example.com/activate?key=7678978978978789 it gets clicked on by the spam filter activating the account and rendering the activation link useless...
I think this is the ongoing issue with invalid activation links that some users are experiencing.

I suggest we move the activate link to a actual button on page that needs to be physically clicked?
My temp fix was to add a deny from all into the .htaccess denying the server access to it's self.

Attachments (1)

6049.diff (6.7 KB) - added by boonebgorges 21 months ago.

Download all attachments as: .zip

Change History (14)

#1 @r-a-y
5 years ago

  • Milestone Awaiting Review deleted
  • Status changed from new to closed

We added a fix for this in #5831. Check out the fix in that ticket if you're interested and see if that works.

Would definitely like to know if that addresses your issue.

Marking this as a duplicate.

Sorry I misread your ticket description, vimes1984!

I think for your particular usecase, you would need to alter the activation email and change the link so it would take you to example.com/activate and add the key on another line in the email.

In order for this to work, you'd need to override the 'bp_core_signup_send_validation_email_message' filter:
https://buddypress.trac.wordpress.org/browser/tags/2.1.1/src/bp-members/bp-members-functions.php#L1938

Last edited 5 years ago by r-a-y (previous) (diff)

#2 @vimes1984
5 years ago

I found the method sending out the emails, that wasn't my problem. It was more a not wanting to edit the core and some users can be a little slow if they have two lines one with the key and one with a link to /activate it's a little complex for people instead of a form with input box say that then requires them to click on a button to confirm it...
I've been reading through a couple of form threads where people are experiencing this issue or similar and I was wondering if it was due to there spam filters...
I was also reading that some email clients do the same.
I have this fixed for me here using a htaccess "deny from X.X.X.X" disallowing the spam filter from reading the outgoing mails, but I also think we could improve this by adding a onpage button validating the users accounts instead of the link providing the validation in itself...
Maybe pass a var through to the onpage form via $_GET which populates a hidden input..
Chris

#3 @r-a-y
5 years ago

  • Component changed from Core to Members
  • Description modified (diff)
  • Keywords dev-feedback added
  • Milestone set to Awaiting Review
  • Status changed from closed to reopened
  • Summary changed from When certain spam filters are installed account gets activated due to spam filters following external link in the email.. to Do not activate user accounts automatically with one click
  • Type changed from defect (bug) to enhancement

Maybe pass a var through to the onpage form via $_GET which populates a hidden input..

That's an interesting idea. I kinda like it!

Instead of the hidden input, I would just populate the existing text field that shows up at example.com/activate/.

Putting this back in the "Awaiting Review" milestone.

#4 @vimes1984
5 years ago

yeah after reading through the other email you could change the name of the var from key to whatever we wanted...

#5 @johnjamesjacoby
5 years ago

  • Keywords needs-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to 2.3

Since our activation form already (incorrectly IMO) uses a get action, this will need some refactoring, but I like the idea of populating the field automatically and sending a proper post action instead.

Moving to 2.3.

#6 @r-a-y
4 years ago

  • Milestone changed from 2.3 to 2.4

#7 @DJPaul
4 years ago

  • Milestone changed from 2.4 to Future Release

#8 @boonebgorges
21 months ago

  • Milestone changed from Future Release to 3.0

I've run into this issue on two different client sites in the past six months. In at least one of the cases, a spammer was aware of the flaw, and was exploiting it by (1) creating accounts using a domain that he knew to be scanned in this way, (2) waiting for the scanner to activate the accounts, and (3) logging into the accounts.

The fix I put in place basically does what @r-a-y suggests above. In the case of the client sites, I also needed to juggle the current_action; presumably, we'd do this differently in BP. https://github.com/livinglab/openlab/commit/caa395eeb4e1d62907ab6363c231fd15cb3510cf

Any objections to changing this behavior in 3.0?

#9 @r-a-y
21 months ago

+1. Let's do it!

#10 @johnjamesjacoby
21 months ago

No objections here.

@boonebgorges
21 months ago

#11 @boonebgorges
21 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

6049.diff is a first pass. It does the following:

  • Removes the auto-activation. Visiting /activate/12345 (or /activate?key=12345 for backward compatibility) prefills the input, and you have to click to submit. The logic is in a separate bp-members-actions.php callback, where I think it makes more sense.
  • Turns the form into a POST rather than a GET. This requires a small bit of back-compat shimming in bp_members_action_activate_account() so that templates using method="get" continue to work.
  • Performs a redirect at the end of a successful activation, so that you don't end up at the same activate/12345 URL (which could result in confusing behavior in some cases). This requires passing an ?activated=1 param, so the template shows the right stuff.
  • Templates are fixed in bp-legacy and bp-nouveau.

#12 @DJPaul
21 months ago

Yeah, that patch looks great!

#13 @boonebgorges
21 months ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from reopened to closed

In 11766:

Members: Require a form submission to activate an account.

Previously, simply loading a URL of the form /activate/12345 would activate
the account with key 12345. This caused conflicts with some mail scanning
services, which follow links in emails, causing accounts to be self-activated.

A small backward-compatibility layer ensures that custom activate.php
templates containing forms with action="get" continue to work.

Fixes #6049.

Note: See TracTickets for help on using tickets.