Don't require login if already authenticated by hash

Thanks. I suggest that we don't even need a config option.

1. The unsubscribe link is a very similar case yet AFAIK does not require login.
2. Requiring someone to log-in before they can unsubscribe is bad UX, perhaps even illegal.
3. If an attacker has access to another users Mail account then they can easily log in anyway with a forgotten password link.

I guess the possible danger case is that I forward my newsletter to someone and they maliciously take the chance to alter my subscription. However I guess that this same risk is present in many major online newsletters.

Ideally this would have a test. It definitely would need one if adding a config option:-).

Updated answer:

The current code insists that users log in to manage their subscription. This is the correct behaviour in almost all cases. NB the code that this patch is altering is on the base subscription form used by all 4 different sub-forms. The only exception to the need to log in is the case that a valid token has been used for SubscriptionsPageForm. In this case SubscriptionsPageForm::buildForm() loaded the entity. Also SubscriptionsAccountForm::buildForm() tries to load an entity. SubscriberForm will always have an entity I would expect. I didn't check all the others but I expect it will be similar. So the correct fix to SubscriptionsFormBase::validateForm() should be to replace the call to \Drupal::currentUser()->isAnonymous() with something that checks if there is a valid entity loaded. There is no need to introduce a config option.

What you have produces the right answer but it can be simpler/clearer. I think that if a user is loaded then the email address is always taken from that user so it can't possibly be wrong. Here is my suggestion:

Starting from the existing code, replace this:
\Drupal::currentUser()->isAnonymous()

with this:
!$this->isAuthenticated()

And add something like this

<strong>protected</strong> function isAuthenticated() {
  $user_loaded = $this->getEntity()->getUser();
  return ($user_loaded && $user_loaded->id());
}

Great thanks. A minor comment:

$user_loaded->id()

would be better as

$user_loaded->isAuthenticated()

Then sorry to be tedious but it would be really great to have a quick test for this case as it would be easy for it to break in future.

Adam is referring to the isAuthenticated method you've added,

+++ b/src/Form/SubscriptionsFormBase.php
@@ -231,6 +231,14 @@ abstract class SubscriptionsFormBase extends ContentEntityForm {
+    return ($user_loaded && $user_loaded->id());

This could be
return ($user_loaded && $user_loaded->isAuthenticated())

Thanks Philipp for your work on this. I have a need for it now so I am happy to finish it off.

Looks good to me.