Template notice not displayed for logged-out users when clicking on email unsubscribe link

[r-a-y]

If a user is logged out and clicks on the Unsubscribe link from one of their BuddyPress emails, the user will land on their profile page and the page is supposed to display a template notice: ​https://github.com/buddypress/buddypress/blob/33e618df29df7126edd6d714147589bdf2370ba1/src/bp-core/bp-core-functions.php#L4594-L4596

But no message is displayed to the user because bp_core_setup_message() only works for logged-in users: ​https://github.com/buddypress/buddypress/blob/33e618df29df7126edd6d714147589bdf2370ba1/src/bp-core/bp-core-functions.php#L1645-L1647

This is still the case in v14.3.4, however interestingly, trunk has removed the is_user_logged_in() clause from bp_core_setup_message():​https://github.com/buddypress/buddypress/blob/8c0406454b260566fb17fc6cb21d755838eac675/src/bp-core/bp-core-functions.php#L1664-L1667, but I cannot find the commit that did this.

Would it make sense to port back trunk's bp_core_setup_message() change to v14-branch?

[r-a-y]

I see r14103 was added to 14-branch and v14.3.4. This does break the email unsubscribe template message.

We're working around this at the moment by altering the email unsubscribe redirect URL to add a special URL parameter to allow the template notice to be displayed, but a core approach would need to take r14103 into consideration.

[dcavins]

Hi @r-a-y, thanks for posting this issue. Yes, that change was made in response to a security change request. The issue was one of those "an unauthenticated user can cause content to be displayed on the screen" issues that get reported a lot these days.

Honestly, we didn't realize that the template messages were ever used for anonymous users, so thanks for pointing out that situation.

The best answer is probably to not rely on cookie data for messages. Do you have any memory when that cookie data would actually be used? Maybe because the user was redirected through the login process or similar and we needed a cheap way to store something specific to the individual temporarily?

Thanks!

[r-a-y]

Do you have any memory when that cookie data would actually be used?

See ​https://github.com/buddypress/buddypress/blob/33e618df29df7126edd6d714147589bdf2370ba1/src/bp-core/bp-core-functions.php#L4594-L4596

bp_core_add_message() is used which sets the cookie data for the unsubscribe message and then the redirect takes place to the members profile URL. The message is then displayed for logged-in users, but for a logged out user, this message doesn't display due to r14103. We need to add back some form of messaging here for logged-out users.

As a tangent, a site that I'm working on is looking into the unsubscribe behavior and preferably, the unsubscribe flow needs a better UX. Should a user really be redirected to the user's profile URL when unsubscribing? The unsubscribe landing page should preferably be a minimal page where you can see what is happening. There should also be a confirmation of the unsubscribe action. Right now, no confirmation occurs as the unsubscribe happens automatically.

We should look into other email unsubscribe workflows and adapt to best practices.

[emaralive]

Is this the only template message of concern or are there others as well?

[emaralive]

Screenshot of message for logged out user - unsubscribe

[emaralive]

What if we did something similar to what is shown in the screenshot for logged-out users?

[r-a-y]

Replying to emaralive:

Is this the only template message of concern or are there others as well?

The registration and activation pages also uses bp_core_add_message(), which would affect logged-out users:

• ​https://github.com/buddypress/buddypress/blob/8c0406454b260566fb17fc6cb21d755838eac675/src/bp-members/screens/register.php#L272
• ​https://github.com/buddypress/buddypress/blob/8c0406454b260566fb17fc6cb21d755838eac675/src/bp-members/screens/activate.php#L112-L125

Edit: The delete account page would also be affected: ​https://github.com/buddypress/buddypress/blob/8c0406454b260566fb17fc6cb21d755838eac675/src/bp-settings/actions/delete-account.php#L50-L58

---

What if we did something similar to what is shown in the screenshot for logged-out users?

I think this helps in the short term for this specific ticket, but we should analyze other unsubscribe workflows to improve the user experience in a future release.

Screenshot removes the link from the "You can change this or any other email notification preferences in your email settings" sentence. We should add it back to make things easier for the user to change their email notification settings if needed.

[emaralive]

Replying to r-a-y:

The registration and activation pages also uses bp_core_add_message(), which would affect logged-out users:...

Thanks for the reply, at the moment, the 3 indicated situations (registration, activation & account deletion) appear to be outside of the scope for this ticket (unsubscribe), I made a ​note to myself to investigate this further.

---

I think this helps in the short term for this specific ticket, but we should analyze other unsubscribe workflows to improve the user experience in a future release.

Screenshot removes the link from the "You can change this or any other email notification preferences in your email settings" sentence. We should add it back to make things easier for the user to change their email notification settings if needed.

The screenshot was just a talking point and I agree there should be a link to the "email notification settings", speaking of which there is a situation whereby the "Account Settings" component is not "activated" which makes the "You can change this or any other email notification preferences in your email settings" sentence nonsensical and should not be included in this case.

Furthermore, there is another situation for "logged-out" users that appears to predate r14103 in which setting the "Community Visibility" option to "Members Only", the unsubscribe notice is not shown due a redirect to the "login" page. And to add more to the pile, for a logged-in user there is the situation for Nouveau template pack with an active "Site Wide" notice, the unsubscribe notice is not shown due the the "Site Wide" notice.

And while looking at long term solutions for the unsubscribe process, the plaintext emails do not contain an unsubscribe link (when it would be applicable) nor do they contain a salutation. When sent via wp_mail(), i.e., plaintext in most cases, there is no opportunity to "unsubscribe".

Since this reply is much longer than I had intended, I'll posit one more situation whereby if a non-member unsubscribes to an invitation, they later decide to register, they won't receive any acknowledgements, e.g., activation email, due to the "opt-out" entry/status; for this one I'm running on memory and will have reconfirm that this situation is true.

[emaralive]

Example of messages for logged-out user - unsubscribe

[emaralive]

The newly added screenshot is what the messages would look like for logged-out users. The examples are just the activity-at-message email type; meaning they adjust according to email type.

[emaralive]

Initial patch

[emaralive]

@dcavins - You expressed interest in having this ticket incorporated into the next release, thus you might want to take a look at the patch and we can take it from there.

[dcavins]

Thanks! I generally like this idea of using the wp_die screen to show the message in certain circumstances. One issue we will have to solve is that with internationalization/localization, the argument in the translation functions like __() must be a string, not a variable, else the potbuilder can't know how to build the dictionary.

In the short term, I would be OK with just using the unsubscribe message without determining whether the user was already unsubscribed or not. "You will no longer receive emails..." is true in either case.

Thanks for working on this!

[emaralive]

Revised patch, replaces 9301.01.diff

[emaralive]

I went with the short term option, since other enhancements can be solved when we engage in a long term solution for the "unsubscribe" process as recommended by @r-a-y. So, if you don't mind, take a look at the revised patch (9301.02.diff).

[emaralive]

Screenshot of messages for logged out user - unsubscribe - revised patch (9301.02.diff)

[emaralive]

Additional screenshot added In the event that someone needed to see how the messages would appear for logged-out users, based on the revised patch (9301.02.diff).

[dcavins]

This looks good to me. Though I wonder if we could simplify the last if/else by adding a logged in check in the outer if condition rather than repeating the wp_die() code. Like:

if ( is_user_logged_in() && $raw_user_id && $redirect_to ) {
		$message = sprintf(
			'%1$s <a href="%2$s">%3$s</a>',
			$result_msg,
			esc_url( $redirect_to ),
			esc_html( $unsub_msg )
		);

		// Template notices are only displayed on BP pages.
		bp_core_add_message( $message );
		bp_core_redirect( bp_members_get_user_url( $raw_user_id ) );

		exit;
	} else {
		wp_die(
			sprintf( '%1$s %2$s', esc_html( $unsub_msg ), esc_html( $result_msg ) ),
			esc_html( $unsub_msg ),
			array(
				'link_url'  => esc_url( home_url() ),
				'link_text' => esc_html__( 'Go to website\'s home page.', 'buddypress' ),
			)
		);
	}

Am I missing a case? Thanks again for coming up with a solution to this problem.

[emaralive]

When I first looked at this, I had the same initial thought until I noticed that the last wp_die() was related to messages without a $redirect_to plus the $unsub_msg and $result_msg are reversed, in addition to having to add a 3rd Augnum for the $redirect_to, i.e., sprintf().

Therefore I opted to not mess with it (in my mind keeping things simple, well, at least for me). Furthermore, any additional revamping would/could/should be the byproduct of a long term solution, which may incorporate:

• Landing page for unsubscribe & the possibility for a resubscribe functionality for sites that have Account Settings inactive.
• The possible implementation of ​RFC 8058 - One-Click Functionality for List Email Headers. Currently, it looks like, ​section 3.2 of RFC 2369 (List-Unsubscribe) has been implemented.
• Other functionalities that would be the result of further analysis when looking at a long term solution.

Having stated the above, sticking a fork in this will free up time for me to work on other tasks. Thoughts?

[dcavins]

Sounds good. I don't feel strongly about it.

Thanks for working on this!

[dcavins]

In 14125:

Improve behavior of bp_email_unsubscribe_handler().

After the changes in the "Improve security of status update messages" changeset, non-logged-in users clicking an unsubscribe link received no feedback on the success of their action.

Props emaralive.

Fixes #9301.

[dcavins]

In 14129:

Improve behavior of bp_email_unsubscribe_handler().

After the changes in the "Improve security of status update messages" changeset, non-logged-in users clicking an unsubscribe link received no feedback on the success of their action.

Props emaralive.

Fixes #9301.

[dcavins]

In 14132:

Improve behavior of bp_email_unsubscribe_handler().

After the changes in the "Improve security of status update messages" changeset, non-logged-in users clicking an unsubscribe link received no feedback on the success of their action.

Props emaralive.

Fixes #9301.

[emaralive]

Initial patch for the 11.0 branch