Merge branch 'sg-dont-require-emails' into invalid_emails_from_github

Author: carols10cents

src/controllers/user/me.rs

@@ -3,6 +3,7 @@ use crate::controllers::prelude::*;
 use crate::controllers::helpers::Paginate;
 use crate::email;
 use crate::util::bad_request;
+use crate::util::errors::CargoError;
 
 use crate::models::{Email, Follow, NewEmail, User, Version};
 use crate::schema::{crates, emails, follows, users, versions};
@@ -127,7 +128,7 @@ pub fn update_user(req: &mut dyn Request) -> CargoResult<Response> {
         return Err(human("empty email rejected"));
     }
 
-    conn.transaction(|| {
+    conn.transaction::<_, Box<dyn CargoError>, _>(|| {
         update(users.filter(gh_login.eq(&user.gh_login)))
             .set(email.eq(user_email))
             .execute(&*conn)?;
@@ -146,8 +147,9 @@ pub fn update_user(req: &mut dyn Request) -> CargoResult<Response> {
             .get_result::<String>(&*conn)
             .map_err(|_| human("Error in creating token"))?;
 
-        crate::email::send_user_confirm_email(user_email, &user.gh_login, &token)
-            .map_err(|_| bad_request("Email could not be sent"))
+        crate::email::send_user_confirm_email(user_email, &user.gh_login, &token);
+
+        Ok(())
     })?;
 
     #[derive(Serialize)]
@@ -199,7 +201,7 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> CargoResult<Response>
             .get_result::<Email>(&*conn)
             .map_err(|_| bad_request("Email could not be found"))?;
 
-        email::send_user_confirm_email(&email.email, &user.gh_login, &email.token)
+        email::try_send_user_confirm_email(&email.email, &user.gh_login, &email.token)
             .map_err(|_| bad_request("Error in sending email"))
     })?;
 

src/email.rs

@@ -57,7 +57,22 @@ fn build_email(
     Ok(email.into())
 }
 
-pub fn send_user_confirm_email(email: &str, user_name: &str, token: &str) -> CargoResult<()> {
+/// Attempts to send a confirmation email. Swallows all errors.
+///
+/// This function swallows any errors that occur while attempting to send the email. Some users
+/// have an invalid email set in their GitHub profile, and we should let them sign in even though
+/// we're trying to silently use their invalid address during signup and can't send them an email.
+/// Use `try_send_user_confirm_email` when the user is directly trying to set their email.
+pub fn send_user_confirm_email(email: &str, user_name: &str, token: &str) {
+    let _ = try_send_user_confirm_email(email, user_name, token);
+}
+
+/// Attempts to send a confirmation email and returns errors.
+///
+/// For use in cases where we want to fail if an email is bad because the user is directly trying
+/// to set their email correctly, as opposed to us silently trying to use the email from their
+/// GitHub profile during signup.
+pub fn try_send_user_confirm_email(email: &str, user_name: &str, token: &str) -> CargoResult<()> {
     // Create a URL with token string as path to send to user
     // If user clicks on path, look email/user up in database,
     // make sure tokens match

src/models/user.rs

@@ -58,7 +58,6 @@ impl<'a> NewUser<'a> {
         use diesel::insert_into;
         use diesel::pg::upsert::excluded;
         use diesel::sql_types::Integer;
-        use diesel::NotFound;
 
         conn.transaction(|| {
             let user = insert_into(users)
@@ -96,8 +95,7 @@ impl<'a> NewUser<'a> {
                     .optional()?;
 
                 if let Some(token) = token {
-                    crate::email::send_user_confirm_email(user_email, &user.gh_login, &token)
-                        .map_err(|_| NotFound)?;
+                    crate::email::send_user_confirm_email(user_email, &user.gh_login, &token);
                 }
             }