I'm not sure when this has changed, but 1.1 works without user being logged in. In our specific case we want to log the user out right after requesting the email address change, because they are logged into several SSO sites with the same email address. With 1.3 we can't use the module because user gets access denied when they try to use the link they got in the email.
It would be nice to support also anonymous users with a link.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mErilainen created an issue. See original summary.

greggles’s picture

Title: Why user has to be logged in? » Why is the user required to be logged in to the same account that requested the change?
Category: Feature request » Support request

Hmmm, I'm not sure when this changed.

Any chance you could go through the commits one-by-one and see when the change was introduced?

mErilainen’s picture

I'd guess it's this one: http://cgit.drupalcode.org/email_confirm/commit/?id=2253329
There is

<?php
if (!isset($uid) || !is_numeric($uid) || !isset($timestamp) || !is_numeric($timestamp) || !isset($hash)) {
 return MENU_ACCESS_DENIED;
}
?>
greggles’s picture

I'm not sure about that - from the referenced issue I talked about keeping that functionality vs. removing it and one scenario why it should be kept https://www.drupal.org/node/2463481#comment-11723967

So, I guess it was present in a prior version than that commit.

purushotam.rai’s picture

Status: Active » Needs review
FileSize
1.64 KB

Status: Needs review » Needs work

The last submitted patch, 5: why_is_the_user-2830292-5.patch, failed testing.

greggles’s picture

@purushotam.rai can you review where this got introduced and why it might have been introduced?

purushotam.rai’s picture

greggles’s picture

It seems unlikely that commit caused the change since it was intended to be about code style and not functional changes. Which line do you think introduced it?

purushotam.rai’s picture

greggles’s picture

Category: Support request » Bug report

Aha! Yes! I wonder....why that change was included there :/

I think it makes sense to revert that one menu related piece of that change.

purushotam.rai’s picture

Status: Needs work » Needs review
FileSize
492 bytes
Ronino’s picture

While #5 doesn't remove the necessity to be logged in for the change confirmation, it helps greatly to improve the user experience:

  1. An anonymous user will now see email_confirm_user_change_mail()'s error messages instead of just "access denied",
  2. it adds the original confirmation URL as destination parameter the login page URL so after logging the confirmation will be done,
  3. it doesn't reject the confirmation anymore if new logins have occurred since the change was made

Patch #12 doesn't work as just reverting the menu item's 'access callback' to TRUE doesn't do the trick here as email_confirm_user_change_mail() will still block the request on line 263.

The commit mentioned in #3 (http://cgit.drupalcode.org/email_confirm/commit/?id=2253329) is correct, but I think it's more on line 263 than on line 248 as $uid is still given for an anonymous user (0). The change was part of the security fix at #2463481: Expirations can be bypassed and username enumeration. While neither the security issue nor the commit log mention anything about the user better being required to be logged in, it's still contained in the commit without documentation why it's done like that.

#85494: Use email verification when changing user email addresses will add email confirmation to D8 and as it seems, the user is not required to be logged in when clicking the link (as #85494-253: Use email verification when changing user email addresses implies).

For comparison: Twitter requires the user to be logged in when confirming the email address change: https://help.twitter.com/en/managing-your-account/how-to-update-your-ema... - I don't know about other services.

brad.bulger’s picture

Agree with #10, the patch in #5 is a preferable fix to the problem.