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

Allow editing your own email address #808

Closed
carols10cents opened this issue Jun 26, 2017 · 7 comments
Closed

Allow editing your own email address #808

carols10cents opened this issue Jun 26, 2017 · 7 comments
Labels
A-accounts C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Comments

@carols10cents
Copy link
Member

We save the email address that we get through github oauth, but you can have multiple email addresses or no email address publicly viewable from your github account.

We don't actually use user email anywhere right now, but before we add anything that might use it (ex: alerts/notifications), we should make it editable (and confirm email address before sending email to it)

@carols10cents
Copy link
Member Author

@natboehm is working on this!

@carols10cents
Copy link
Member Author

Here's how npm is rolling out email verification: http://blog.npmjs.org/post/163187838935/upcoming-change-verified-email-required?utm_campaign=newsletter20170720&utm_source=newsletter_mailer&utm_medium=email

@carols10cents
Copy link
Member Author

@natboehm there were some questions about what the issues are around email and how we're planning to implement emails over in #924. Could you please write up what we've talked about? Here are some questions from over there too:

  • Also to address the email thing, when I was working with the passport-github package, I found out that they actually do a separate API call to fetch the email after authentication. Maybe that could work as a longer term solution for getting emails? It's not ideal, but it does seem to work.

  • By mailer service do you mean something that sends emails? What about using something existing like mailgun? The pricing is really great even at scale.

  • Maybe we can even figure out a way to fetch emails lazily (and then store them) as we need to.

  • To prevent being spammy, it might be prudent to also ask people if we can email them at all.

@natboehm
Copy link
Contributor

natboehm commented Aug 1, 2017

The plan for user emails we've come up with is:

  1. Add a field to edit a user's email address
  2. Ability to send a confirmation email to the user for verification
  3. Figure out a way to make adding an email mandatory

(1) is almost completed in PR #921.

For (2), the plan was to send the user a verification email on the addition or edit of their email address. We would generate a token in the database and send in the email as part of a link. Once the user clicks the link, we would look up by the token, see if the user matches, and if the account is found, the email is confirmed. I've been looking into doing this yesterday and today, and found the crate lettre that might be helpful used in combination with the Heroku app Mailgun to be able to send an email from the app to the user.

For step (3), we've talked about a few options. One would be require users to confirm their emails when publishing a crate. Another would be on account creation or next time logging in. More extreme would be to disable accounts until a valid email is provided and confirmed. I could see using a combination of these options to make sure no one slips through an unforeseen loophole. We also might want to figure out a way to make sure that the email received from GitHub is an active account/valid email.

Questions from #924:

Also to address the email thing, when I was working with the passport-github package, I found out that they actually do a separate API call to fetch the email after authentication. Maybe that could work as a longer term solution for getting emails? It's not ideal, but it does seem to work.

@carols10cents and I looked into this and found that you're right, that does work. Even though it says on GitHub that your email is private, everything seems private in the email settings, with the proper authentication you can query https://api.github.com/user/emails and get all emails associated with an account, public or private. While this would seemingly allow us to get all user emails, we decided that we'd rather not go this route. This would rely too much on the GitHub API for some potentially critical user information. GitHub also allows users to have multiple email accounts associated, so it could be difficult to know which address is the best way to contact a user. Personally, I was unaware that my email on GitHub was not actually private, only private to the web interface with other applications able to access it, so other users may be uncomfortable that we were able to get what they thought were their private emails. While adding an email will be mandatory, it might feel better for users to know what they are adding than to grab something they might not know about.

By mailer service do you mean something that sends emails? What about using something existing like mailgun? The pricing is really great even at scale.

By mailer service, we do mean something that sends emails. As mentioned above, I've actually been looking into using Mailgun with the lettre crate in hopes of making this easier to implement. If whoever asked this has experience using Mailgun I'd love to hear more about it!

Maybe we can even figure out a way to fetch emails lazily (and then store them) as we need to.

If by fetching lazily and storing as needed you mean getting emails when needed and storing them, it is problematic that if don't have a user's email, we don't have a way of contacting them in the first place to then get their email. We could deactivate their account and require them to add their email the next time they log in or publish to get their account, but that seems unnecessarily unfriendly. For our use case I don't think fetching emails lazily would work.

Conversely, if you were referring to the process of getting user emails as lazy, I would say that is what we are likely going to want to do. We wouldn't want to immediately deactivate all user accounts lacking an email, we would want to perhaps first require it on crate publish, then on updating a crate's version, account creation, etc.

To prevent being spammy, it might be prudent to also ask people if we can email them at all.

I'd agree with that, anything after the initial confirmation email and other than critical emails should probably be opt-in. If we have the ability to confirm a user as a crate owner on the web UI, receiving an email in addition probably isn't necessary but could be an option if desired.

bors-voyager bot added a commit that referenced this issue Aug 2, 2017
921: Ability to add/update user email r=carols10cents

This PR addresses issue #808, allow editing your own email address. This is the first step in a three part implementation, adding a field to edit an email address. Yet to be implemented before the issue is resolved is (1) the ability to verify emails with users and (2) figuring out how to get everyone to add their email to their account. This PR only implements the addition of a button to the email field such that a user's email can be added or updated, as well as hiding the email field from public view by separating `EncodableUser` into `EncodablePublicUser` and `EncodablePrivateUser`.
@vignesh-sankaran
Copy link
Contributor

Is this still being worked on @natboehm?

@carols10cents
Copy link
Member Author

Yes, she's still working on this.

bors-voyager bot added a commit that referenced this issue Sep 15, 2017
1045: Sending an email to a user to confirm their email address r=carols10cents

This PR addresses issue #808, allow editing your own email address. Following the discussion on the issue, this implements the second of three parts, the ability to send a confirmation email to a user for verification. For a full explanation of the issue, see [this comment](#808 (comment)) I left which should fully explain the changes made, as well as parts 1 and 3. 

In short, I added two tables to the database: one for the user's email, the other for a confirmation token associated with the email. Once the user clicks the link sent, the associated token is deleted from the token table and `email_verified` is set to `true` in the email table. It is indicated to the user if they have not verified their email on the account settings page, a message and email resend button being displayed. Sending emails requires Mailgun to be added to the crates.io Heroku account. 

This has applications for issue #924 in that we should now be able to send an email to a user, if they have added their email. I know there was a lot of discussion on that issue regarding the ability to send users an email to be added as an owner.
bors-voyager bot added a commit that referenced this issue Sep 15, 2017
1045: Sending an email to a user to confirm their email address r=carols10cents

This PR addresses issue #808, allow editing your own email address. Following the discussion on the issue, this implements the second of three parts, the ability to send a confirmation email to a user for verification. For a full explanation of the issue, see [this comment](#808 (comment)) I left which should fully explain the changes made, as well as parts 1 and 3. 

In short, I added two tables to the database: one for the user's email, the other for a confirmation token associated with the email. Once the user clicks the link sent, the associated token is deleted from the token table and `email_verified` is set to `true` in the email table. It is indicated to the user if they have not verified their email on the account settings page, a message and email resend button being displayed. Sending emails requires Mailgun to be added to the crates.io Heroku account. 

This has applications for issue #924 in that we should now be able to send an email to a user, if they have added their email. I know there was a lot of discussion on that issue regarding the ability to send users an email to be added as an owner.
bors-voyager bot added a commit that referenced this issue Sep 15, 2017
1045: Sending an email to a user to confirm their email address r=carols10cents

This PR addresses issue #808, allow editing your own email address. Following the discussion on the issue, this implements the second of three parts, the ability to send a confirmation email to a user for verification. For a full explanation of the issue, see [this comment](#808 (comment)) I left which should fully explain the changes made, as well as parts 1 and 3. 

In short, I added two tables to the database: one for the user's email, the other for a confirmation token associated with the email. Once the user clicks the link sent, the associated token is deleted from the token table and `email_verified` is set to `true` in the email table. It is indicated to the user if they have not verified their email on the account settings page, a message and email resend button being displayed. Sending emails requires Mailgun to be added to the crates.io Heroku account. 

This has applications for issue #924 in that we should now be able to send an email to a user, if they have added their email. I know there was a lot of discussion on that issue regarding the ability to send users an email to be added as an owner.
@carols10cents
Copy link
Member Author

I think this can be closed now! 🎉

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works and removed C-feature-request labels Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-accounts C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

No branches or pull requests

4 participants