worker/jobs/sync_admins: Send notification emails

Author: Turbo87

Cargo.lock

@@ -129,4 +129,5 @@ crates_io_test_db = { path = "crates_io_test_db" }
 claims = "=0.7.1"
 googletest = "=0.10.0"
 insta = { version = "=1.34.0", features = ["json", "redactions"] }
+regex = "=1.10.2"
 tokio = "=1.35.1"

Cargo.toml

@@ -0,0 +1,8 @@
+---
+source: src/tests/worker/sync_admins.rs
+expression: emails
+---
+[
+    "To: existing-admin@crates.io\r\nFrom: noreply@crates.io\r\nSubject: crates.io: Admin account changes\r\nContent-Type: text/plain; charset=utf-8\r\nContent-Transfer-Encoding: 7bit\r\n\r\nNew admins have been added:\r\n\r\n- new-admin (github_id: 3)\r\n\r\nAdmin access has been revoked for:\r\n- obsolete-admin (github_id: 2)\r\n",
+    "To: obsolete-admin@crates.io\r\nFrom: noreply@crates.io\r\nSubject: crates.io: Admin account changes\r\nContent-Type: text/plain; charset=utf-8\r\nContent-Transfer-Encoding: 7bit\r\n\r\nNew admins have been added:\r\n\r\n- new-admin (github_id: 3)\r\n\r\nAdmin access has been revoked for:\r\n- obsolete-admin (github_id: 2)\r\n",
+]

src/tests/worker/snapshots/all__worker__sync_admins__sync_admins_job.snap

@@ -1,10 +1,12 @@
 use crate::util::TestApp;
-use crates_io::schema::users;
+use crates_io::schema::{emails, users};
 use crates_io::team_repo::{Member, MockTeamRepo, Team};
 use crates_io::worker::jobs::SyncAdmins;
 use crates_io_worker::BackgroundJob;
 use diesel::prelude::*;
 use diesel::{PgConnection, QueryResult, RunQueryDsl};
+use insta::assert_debug_snapshot;
+use regex::Regex;
 
 #[test]
 fn test_sync_admins_job() {
@@ -39,6 +41,15 @@ fn test_sync_admins_job() {
     let admins = app.db(|conn| get_admins(conn).unwrap());
     let expected_admins = vec![("existing-admin".into(), 1), ("new-admin".into(), 3)];
     assert_eq!(admins, expected_admins);
+
+    let email_header_regex = Regex::new(r"(Message-ID|Date): [^\r\n]+\r\n").unwrap();
+    let emails = app.as_inner().emails.mails_in_memory().unwrap();
+    let emails = emails
+        .iter()
+        .map(|(_, email)| email_header_regex.replace_all(email, ""))
+        .collect::<Vec<_>>();
+
+    assert_debug_snapshot!(emails);
 }
 
 fn mock_team(name: impl Into<String>, members: Vec<Member>) -> Team {
@@ -61,14 +72,23 @@ fn mock_member(name: impl Into<String>, github_id: i32) -> Member {
 }
 
 fn create_user(name: &str, gh_id: i32, is_admin: bool, conn: &mut PgConnection) -> QueryResult<()> {
-    diesel::insert_into(users::table)
+    let user_id = diesel::insert_into(users::table)
         .values((
             users::name.eq(name),
             users::gh_login.eq(name),
             users::gh_id.eq(gh_id),
             users::gh_access_token.eq("some random token"),
             users::is_admin.eq(is_admin),
         ))
+        .returning(users::id)
+        .get_result::<i32>(conn)?;
+
+    diesel::insert_into(emails::table)
+        .values((
+            emails::user_id.eq(user_id),
+            emails::email.eq(format!("{}@crates.io", name)),
+            emails::verified.eq(true),
+        ))
         .execute(conn)?;
 
     Ok(())

src/tests/worker/sync_admins.rs

@@ -1,10 +1,12 @@
-use crate::schema::users;
+use crate::email::Email;
+use crate::schema::{emails, users};
 use crate::tasks::spawn_blocking;
 use crate::worker::Environment;
 use crates_io_worker::BackgroundJob;
 use diesel::prelude::*;
 use diesel::RunQueryDsl;
 use std::collections::HashSet;
+use std::fmt::{Display, Formatter};
 use std::sync::Arc;
 
 /// See <https://github.com/rust-lang/team/blob/master/teams/crates-io-admins.toml>.
@@ -31,57 +33,83 @@ impl BackgroundJob for SyncAdmins {
             let mut conn = ctx.connection_pool.get()?;
 
             let database_admins = users::table
-                .select((users::gh_id, users::gh_login))
+                .left_join(emails::table)
+                .select((users::gh_id, users::gh_login, emails::email.nullable()))
                 .filter(users::is_admin.eq(true))
-                .get_results::<(i32, String)>(&mut conn)?;
+                .get_results::<(i32, String, Option<String>)>(&mut conn)?;
 
             let database_admin_ids = database_admins
                 .iter()
-                .map(|(gh_id, _)| *gh_id)
+                .map(|(gh_id, _, _)| *gh_id)
                 .collect::<HashSet<_>>();
 
             let new_admin_ids = repo_admin_ids
                 .difference(&database_admin_ids)
                 .collect::<HashSet<_>>();
 
-            if new_admin_ids.is_empty() {
+            let new_admins = if new_admin_ids.is_empty() {
                 debug!("No new admins to add");
+                vec![]
             } else {
                 let new_admins = repo_admins
                     .iter()
                     .filter(|m| new_admin_ids.contains(&&m.github_id))
                     .map(|m| format!("{} (github_id: {})", m.github, m.github_id))
-                    .collect::<Vec<_>>()
-                    .join(", ");
+                    .collect::<Vec<_>>();
 
-                info!("Adding new admins: {}", new_admins);
+                info!("Adding new admins: {}", new_admins.join(", "));
 
                 diesel::update(users::table)
                     .filter(users::gh_id.eq_any(new_admin_ids))
                     .set(users::is_admin.eq(true))
                     .execute(&mut conn)?;
-            }
+
+                new_admins
+            };
 
             let obsolete_admin_ids = database_admin_ids
                 .difference(&repo_admin_ids)
                 .collect::<HashSet<_>>();
 
-            if obsolete_admin_ids.is_empty() {
+            let obsolete_admins = if obsolete_admin_ids.is_empty() {
                 debug!("No obsolete admins to remove");
+                vec![]
             } else {
                 let obsolete_admins = database_admins
                     .iter()
-                    .filter(|(gh_id, _)| obsolete_admin_ids.contains(&gh_id))
-                    .map(|(gh_id, login)| format!("{} (github_id: {})", login, gh_id))
-                    .collect::<Vec<_>>()
-                    .join(", ");
+                    .filter(|(gh_id, _, _)| obsolete_admin_ids.contains(&gh_id))
+                    .map(|(gh_id, login, _)| format!("{} (github_id: {})", login, gh_id))
+                    .collect::<Vec<_>>();
 
-                info!("Removing obsolete admins: {}", obsolete_admins);
+                info!("Removing obsolete admins: {}", obsolete_admins.join(", "));
 
                 diesel::update(users::table)
                     .filter(users::gh_id.eq_any(obsolete_admin_ids))
                     .set(users::is_admin.eq(false))
                     .execute(&mut conn)?;
+
+                obsolete_admins
+            };
+
+            if !new_admins.is_empty() || !obsolete_admins.is_empty() {
+                let email = AdminAccountEmail::new(new_admins, obsolete_admins);
+
+                for database_admin in &database_admins {
+                    let (_, _, email_address) = database_admin;
+                    if let Some(email_address) = email_address {
+                        if let Err(error) = ctx.emails.send(email_address, email.clone()) {
+                            warn!(
+                                "Failed to send email to admin {} ({}, github_id: {}): {}",
+                                database_admin.1, email_address, database_admin.0, error
+                            );
+                        }
+                    } else {
+                        warn!(
+                            "No email address found for admin {} (github_id: {})",
+                            database_admin.1, database_admin.0
+                        );
+                    }
+                }
             }
 
             Ok(())
@@ -91,3 +119,47 @@ impl BackgroundJob for SyncAdmins {
         Ok(())
     }
 }
+
+#[derive(Debug, Clone)]
+struct AdminAccountEmail {
+    new_admins: Vec<String>,
+    obsolete_admins: Vec<String>,
+}
+
+impl AdminAccountEmail {
+    fn new(new_admins: Vec<String>, obsolete_admins: Vec<String>) -> Self {
+        Self {
+            new_admins,
+            obsolete_admins,
+        }
+    }
+}
+
+impl Email for AdminAccountEmail {
+    const SUBJECT: &'static str = "crates.io: Admin account changes";
+
+    fn body(&self) -> String {
+        self.to_string()
+    }
+}
+
+impl Display for AdminAccountEmail {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        if !self.new_admins.is_empty() {
+            writeln!(f, "New admins have been added:\n")?;
+            for new_admin in &self.new_admins {
+                writeln!(f, "- {}", new_admin)?;
+            }
+            writeln!(f)?;
+        }
+
+        if !self.obsolete_admins.is_empty() {
+            writeln!(f, "Admin access has been revoked for:")?;
+            for obsolete_admin in &self.obsolete_admins {
+                writeln!(f, "- {}", obsolete_admin)?;
+            }
+        }
+
+        Ok(())
+    }
+}