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

Jump to bottom

Do not send password changed email when triggered by API #14267

Closed
futureweb opened this issue Mar 25, 2019 · 6 comments
Closed

Do not send password changed email when triggered by API #14267

futureweb opened this issue Mar 25, 2019 · 6 comments
Assignees
@tsteur @katebutler
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone
3.10.0

Comments

@futureweb
Copy link
Contributor

futureweb commented Mar 25, 2019 •
edited

When Password change is triggered by API "UsersManager.updateUser" Method (confirm PW = ADMIN PW) no "password changed" E-Mail should be sent.

Reason: We for our case got Matomo integrated into our SAAS CMS with automatic PW changes over the API. Since 3.9.1 thousands of our Customers getting double Mails ... one from our CMS and one from Matomo - resulting in countless Support Calls / Mails.

Related to: #14240 (Do not send password changed email for automated use cases)

Forum Reference: https://forum.matomo.org/t/3-9-1-password-changed-e-mail-on-automated-password-changes-by-api/32262

thx
Andreas Schnederle-Wagner
@tsteur
Copy link
Member

tsteur commented Mar 25, 2019 •
edited

Makes sense 👍

@tsteur tsteur added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Mar 25, 2019
@tsteur tsteur added this to the 3.10.0 milestone Mar 25, 2019
@tsteur
Copy link
Member

tsteur commented Mar 25, 2019

Could be maybe an API parameter optionally that allows to send email but maybe not needed.

@tsteur
Copy link
Member

tsteur commented Mar 27, 2019

Actually, best would be to add an API parameter for whether to send an email or not and our UI that issues the API would force sending the email.

@katebutler katebutler self-assigned this Mar 27, 2019
@Findus23
Copy link
Member

Makes sense, but this also raises the new issue that this kind of circumvents the original purpose (or at least one of them) of the feature (detect when an attacker tries to change your password).

If someone can simply change the password without sending the E-Mail then the E-Mail doesn't really carry that much information anymore.

Of course one could make this parameter only available to admin users but still then admin accounts wouldn't really be protected.

Honestly I am not sure how other websites/webapps are implementing this.

@tsteur
Copy link
Member

tsteur commented Mar 27, 2019

I think it's fine to have the option to disable it through the API. Alternatively we could add a new config setting but this is hard for users that can't change the config option (eg on cloud). @mattab any thoughts?

@mattab
Copy link
Member

mattab commented Mar 27, 2019

  • +1 for a new config file setting to disable the email notifications. (Config file settings can't be changed through the UI so an attacker couldn't disable it and it should keep the feature secure.)
  • we can document the setting in a new FAQ "How do I disable the email notifications when password is changed and email address is changed?"

@simivar simivar mentioned this issue Mar 28, 2019
Merged
tsteur pushed a commit that referenced this issue Apr 11, 2019
@simivar @tsteur
Add a config to disable e-mail notifications on password/email change…
4566c44 
… through API (#14281)

* Add a config to disable e-mail notifications on password/email change through API #14267

* Add tests for api_update_users_email_notifications setting

* rename setting

* Update API.php

* Update APITest.php

* move enable_update_users_email=1 to teardown in case the test fails
@tsteur tsteur closed this as completed Apr 11, 2019
@tsteur tsteur self-assigned this Apr 11, 2019
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jun 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsteur tsteur

@katebutler katebutler

Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
3.10.0
Development

No branches or pull requests
5 participants
@tsteur @mattab @futureweb @katebutler @Findus23