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

Web RPC am_set_info should not email user when email changes. #2565

Open
drshawnkwang opened this issue Jun 21, 2018 · 5 comments
Open

Web RPC am_set_info should not email user when email changes. #2565

drshawnkwang opened this issue Jun 21, 2018 · 5 comments

Comments

@drshawnkwang
Copy link
Contributor

Web RPC am_set_info.php was altered in PR #2500 to add the following lines of code.

if ($send_changed_email_to_user) {
    send_changed_email($user);
}

The function send_changed_email() sends the two emails to the current and previous email addresses for the user.

I think this code should be removed/reverted, because this ‘layer’ of the code is the wrong place to send emails to a user. A good example is if someone is using an Account Manager, is attached to ten projects, and then uses the Account Manager interface to change his/her email. (Assuming all 10 projects are using this code;) The user will receive ten emails from all the projects. The AM should be the layer which handles the email exchange.

Likewise, in the Drupal-BOINC implementation, Drupal is handing the emails to the user, with a different link for recovering one’s email address. If a someone uses this RPC to change his/her email, then s/he will receive an email with the incorrect link for the Drupal run Website.

I would ask @Uplinger and @TheAspens to chime in because they developed this functionality.

@drshawnkwang drshawnkwang changed the title Web RPC am_set_info should not email use when email changes. Web RPC am_set_info should not email user when email changes. Jun 21, 2018
@davidpanderson
Copy link
Contributor

A hacker could steal a user's authenticator,
then send a web RPC to change their email address.
With this change, the user wouldn't be notified.

@tristanolive
Copy link
Contributor

Perhaps this depends on #2308, which would restrict access to web RPCs (or rather those specific to account management) to a whitelist of vetted account managers. If the private key approach mentioned in that issue is too involved, I think basing the whitelist on IP addresses seems reasonable for now.

@davidpanderson
Copy link
Contributor

Sender IP address is not secure.
I don't think this is worth doing in the first place.

@drshawnkwang
Copy link
Contributor Author

The more I think about this, the more I am convinced there are two problems that overlap:

  1. Account Managers (AM) need to be able to interact with a project, to be able to change an email address or a project preference, etc. This needs to be done via Web RPC calls. Users who use AM expect that the AM take care of everything for them.
  2. Projects need to be able to inform users when emails change, because a malicious user could use hijack someone's account. Individual projects want to be able to have control of their own user base and not be exposed to malicious use.

What if the am_set_info.php RPC was changed: when the AM changes the user's email, all the projects that are involved send one email to the user's previous email address. This would be different that the ones it currently sends. Example text: "An account manager has changed your email project Foobar@Home. If this was not your intent please ... (do something something, which may or may not contain the recovery link)". Account managers could have the text "Please note you will receive an email from each and every project you are attached to when you change your email address." to warn the user of the impending communiques.

Admittedly the user would receive N emails (N= number of projects) saying the same thing, but this way if a malicious actor hijacks the email, the user is at least notified.

@Uplinger
Copy link
Contributor

Uplinger commented Jul 3, 2018

Hey Shawn,

Thanks for writing this up. Let me make sure I'm understanding your thoughts from an account manager perspective.

When someone changes their email in the account manager, they only receive a single email to their old email address from EACH project that their email address has been changed by an account manager. You do not wish to have the new email address getting an email from each project clarifying that they changed the email address.

I personally think the new email still needs to be sent, this is because not every project will be updated to the email notification scheme. This means that if they change their email today and try again tomorrow, only projects running the old code will update the email address again. The projects running the new code will respond with an error to the account managers. This could be confusing to the members if they didn't receive the email letting them know they can only change their email after 7 days.

I think the extra information is valuable from an individual project perspective, as it clarifies things to the end user as well as helps protect them from malicious attempts. The subjects will be uniform which would help them see that a mass change has happened, but that is just a minor delete of extra messages if they warranted the change.

Note: We can change the subject to let them know that instead of the subject being:

PROJECT." email address change"

Something like this can be set when coming from the account manager (unfortunately I dont' think account managers pass their name to the server when connecting):

"An Account Manager has changed your email address on ".PROJECT

Thanks,
-Keith

@Ageless93 Ageless93 added this to Backlog in Other via automation Jul 12, 2018
@AenBleidd AenBleidd added this to Backlog in Server via automation Oct 30, 2019
@AenBleidd AenBleidd added this to Backlog in Website and Drupal via automation Oct 30, 2019
@AenBleidd AenBleidd removed this from Backlog in Other Oct 30, 2019
@AenBleidd AenBleidd added this to the Server milestone Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Status: Backlog
Server
  
Backlog
Development

No branches or pull requests

6 participants