Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mastodon "Email changed" email does not show the new email address #6778

Closed
ayush-sharma opened this issue Mar 13, 2018 · 3 comments · Fixed by #13475
Closed

Mastodon "Email changed" email does not show the new email address #6778

ayush-sharma opened this issue Mar 13, 2018 · 3 comments · Fixed by #13475

Comments

@ayush-sharma
Copy link

I recently changed my Mastodon email address, and I got a confirmation email on my old email address saying it was changed. The email did not show the new email address (see attachment). The email does mention the the following:

"If you did not change your email, it is likely that someone has gained access to your account."

Would be nice to know who it was.

Instance: mastodon.technology
Browser: Safari + Mac OS X (High Sierra)
Incident time: 12/3/18 1:03 AM (IST)

screenshot- 2018-03-12 at 1 04 28 am

ClearlyClaire added a commit to ClearlyClaire/mastodon that referenced this issue Mar 19, 2018
smorimoto pushed a commit to kibousoft/mastodon that referenced this issue Apr 26, 2018
@ayush-sharma
Copy link
Author

Hi again.

I'd this issue re-opened because I think its unresolved. I wanted to check so I updated my email address, and this is the email I received:

screenshot- 2018-07-13 at 5 51 15 am

This email arrived at my already registered email address, and says that I'm changing it to my already registered email address, and does not show me the new email.

Also, the colour of the link is not really readable on my Retina display with low brightness, and seems to vibrate. Not to gripe, but can it be made lighter?

Hope this helps.

@ayush-sharma
Copy link
Author

The Mastodon version was 2.4.0.

@ayush-sharma
Copy link
Author

@Gargron

@ClearlyClaire ClearlyClaire reopened this Apr 14, 2020
ClearlyClaire added a commit to ClearlyClaire/mastodon that referenced this issue Apr 15, 2020
Fixes mastodon#6778

The root of the issue is that `send_devise_notification` was called before
the changes were properly commited to the database, causing the mailer to
pick previous values if running too early.

Devise's documentation provides guidance on how to handle that[1][2], however,
I have found it to not be working, as the following happens, in that order:
- `send_devise_notification` is called for the `email_changed` notification.
  In that case, `changed?` is false and `saved_changes?` is true, so
  if we use the former, we have the same issue.
- the `after_commit` hook is called
- `send_devise_notification` is called for the `confirmation_instructions`
  notification.
  In that case, `changed?` is still false, and `saved_changes?` still true,
  so if we use the latter, that second notification email is simply not
  going to be sent (as we would be queuing the notification *after*
  executing the after_commit hook).

This is because it may be called from either an `after_update` or
`after_commit` hook, the difference not being a call to `save` but the
transaction actually being committed to the database. This may arguably
be a bug in Devise, or Devise's notification.

The proposed workaround is inspired by Devise's documentation but checks
whether a transaction is open to make the call whether to immediately
send the notification or defer it to the `after_commit` hook.

[1]: https://www.rubydoc.info/github/plataformatec/devise/Devise%2FModels%2FAuthenticatable:send_devise_notification
[2]: https://github.com/heartcombo/devise/blob/406915cb781e38255a30ad2a0609e33952b9ec50/lib/devise/models/authenticatable.rb#L133-L194
Gargron pushed a commit that referenced this issue Apr 15, 2020
* Fix “Email changed” notification sometimes having wrong e-mail

Fixes #6778

The root of the issue is that `send_devise_notification` was called before
the changes were properly commited to the database, causing the mailer to
pick previous values if running too early.

Devise's documentation provides guidance on how to handle that[1][2], however,
I have found it to not be working, as the following happens, in that order:
- `send_devise_notification` is called for the `email_changed` notification.
  In that case, `changed?` is false and `saved_changes?` is true, so
  if we use the former, we have the same issue.
- the `after_commit` hook is called
- `send_devise_notification` is called for the `confirmation_instructions`
  notification.
  In that case, `changed?` is still false, and `saved_changes?` still true,
  so if we use the latter, that second notification email is simply not
  going to be sent (as we would be queuing the notification *after*
  executing the after_commit hook).

This is because it may be called from either an `after_update` or
`after_commit` hook, the difference not being a call to `save` but the
transaction actually being committed to the database. This may arguably
be a bug in Devise, or Devise's notification.

The proposed workaround is inspired by Devise's documentation but checks
whether a transaction is open to make the call whether to immediately
send the notification or defer it to the `after_commit` hook.

[1]: https://www.rubydoc.info/github/plataformatec/devise/Devise%2FModels%2FAuthenticatable:send_devise_notification
[2]: https://github.com/heartcombo/devise/blob/406915cb781e38255a30ad2a0609e33952b9ec50/lib/devise/models/authenticatable.rb#L133-L194

* Fix cases when sending notifications without changing the model

* Defer sending if and only if in transaction including current record
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants