Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#12 | email-confirm-user-login-req-2830292.patch | 492 bytes | purushotam.rai |
| |||
#10 | Screen Shot 2017-12-05 at 1.33.44 AM.png | 228.24 KB | purushotam.rai |
#5 | why_is_the_user-2830292-5.patch | 1.64 KB | purushotam.rai |
Issue fork email_confirm-2830292
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:
Comments
Comment #2
gregglesHmmm, 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?
Comment #3
mErilainen CreditAttribution: mErilainen at Wunder commentedI'd guess it's this one: http://cgit.drupalcode.org/email_confirm/commit/?id=2253329
There is
Comment #4
gregglesI'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.
Comment #5
purushotam.rai CreditAttribution: purushotam.rai as a volunteer and at QED42 commentedComment #7
greggles@purushotam.rai can you review where this got introduced and why it might have been introduced?
Comment #8
purushotam.rai CreditAttribution: purushotam.rai as a volunteer and at QED42 commented@greggles, I've found the commit: http://cgit.drupalcode.org/email_confirm/commit/?id=9f7ec018777f5c9d78e8...
and the issue was https://www.drupal.org/project/email_confirm/issues/2818549
Hope this helps
Comment #9
gregglesIt 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?
Comment #10
purushotam.rai CreditAttribution: purushotam.rai as a volunteer and at QED42 commentedComment #11
gregglesAha! Yes! I wonder....why that change was included there :/
I think it makes sense to revert that one menu related piece of that change.
Comment #12
purushotam.rai CreditAttribution: purushotam.rai as a volunteer and at QED42 commentedComment #13
Ronino CreditAttribution: Ronino as a volunteer commentedWhile #5 doesn't remove the necessity to be logged in for the change confirmation, it helps greatly to improve the user experience:
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.
Comment #14
brad.bulger CreditAttribution: brad.bulger commentedAgree with #10, the patch in #5 is a preferable fix to the problem.