diff --git a/src/background_jobs.rs b/src/background_jobs.rs index daa34bf463d..e20dba6e41c 100644 --- a/src/background_jobs.rs +++ b/src/background_jobs.rs @@ -6,6 +6,7 @@ use diesel::r2d2::PoolError; use swirl::PerformError; use crate::db::{DieselPool, DieselPooledConn}; +use crate::email::Emails; use crate::git::Repository; use crate::uploaders::Uploader; @@ -26,6 +27,7 @@ pub struct Environment { index: Arc>, pub uploader: Uploader, http_client: AssertUnwindSafe, + pub emails: Arc, } impl Clone for Environment { @@ -34,24 +36,32 @@ impl Clone for Environment { index: self.index.clone(), uploader: self.uploader.clone(), http_client: AssertUnwindSafe(self.http_client.0.clone()), + emails: self.emails.clone(), } } } impl Environment { - pub fn new(index: Repository, uploader: Uploader, http_client: Client) -> Self { - Self::new_shared(Arc::new(Mutex::new(index)), uploader, http_client) + pub fn new(index: Repository, uploader: Uploader, http_client: Client, emails: Emails) -> Self { + Self::new_shared( + Arc::new(Mutex::new(index)), + uploader, + http_client, + Arc::new(emails), + ) } pub fn new_shared( index: Arc>, uploader: Uploader, http_client: Client, + emails: Arc, ) -> Self { Self { index, uploader, http_client: AssertUnwindSafe(http_client), + emails, } } diff --git a/src/bin/background-worker.rs b/src/bin/background-worker.rs index b2bcb39407d..2d556917c2b 100644 --- a/src/bin/background-worker.rs +++ b/src/bin/background-worker.rs @@ -12,6 +12,7 @@ #![warn(clippy::all, rust_2018_idioms)] +use cargo_registry::email::Emails; use cargo_registry::git::{Repository, RepositoryConfig}; use cargo_registry::{background_jobs::*, db}; use diesel::r2d2; @@ -39,9 +40,16 @@ fn main() { )); println!("Index cloned"); + let emails = Emails::from_environment(); + let emails = Arc::new(emails); + let build_runner = || { - let environment = - Environment::new_shared(repository.clone(), config.uploader.clone(), Client::new()); + let environment = Environment::new_shared( + repository.clone(), + config.uploader.clone(), + Client::new(), + emails.clone(), + ); let db_config = r2d2::Pool::builder().min_idle(Some(0)); swirl::Runner::builder(environment) .connection_pool_builder(&db_url, db_config) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 80a5506ea86..49858f35b9d 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -204,7 +204,8 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult { let hex_cksum = cksum.encode_hex::(); - // Register this crate in our local git repo. + // Register this crate in our local git repo and send notification emails + // to owners who haven't opted out of them. let git_crate = git::Crate { name: name.0, vers: vers.to_string(), @@ -214,7 +215,8 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult { yanked: Some(false), links, }; - git::add_crate(git_crate).enqueue(&conn)?; + let emails = krate.owners_with_notification_email(&conn)?; + git::add_crate(git_crate, emails, user.name, verified_email_address).enqueue(&conn)?; // The `other` field on `PublishWarnings` was introduced to handle a temporary warning // that is no longer needed. As such, crates.io currently does not return any `other` diff --git a/src/email.rs b/src/email.rs index 201c7f1cd23..b70513b69c2 100644 --- a/src/email.rs +++ b/src/email.rs @@ -61,7 +61,7 @@ https://{}/confirm/{}", token ); - self.send(email, subject, &body) + self.send(&[email], subject, &body) } /// Attempts to send an ownership invitation. @@ -83,7 +83,45 @@ or go to https://{domain}/me/pending-invites to manage all of your crate ownersh domain = crate::config::domain_name() ); - self.send(email, subject, &body) + self.send(&[email], subject, &body) + } + + /// Attempts to send a new crate version published notification to crate owners. + pub fn notify_owners( + &self, + emails: &[&str], + crate_name: &str, + crate_version: &str, + publisher_name: Option<&str>, + publisher_email: &str, + ) -> AppResult<()> { + let subject = format!( + "Crate {} ({}) published to {}", + crate_name, + crate_version, + crate::config::domain_name() + ); + let body = format!( + "A crate you have publish access to has recently released a new version. +Crate: {} ({}) +Published by: {} <{}> +Published at: {} +If this publish is expected, you do not need to take further action. +Only if this publish is unexpected, please take immediate steps to secure your account: +* If you suspect your GitHub account was compromised, change your password +* Revoke your API Token +* Yank the version of the crate reported in this email +* Report this incident to RustSec https://rustsec.org +To stop receiving these messages, update your email notification settings at https://{domain}/me", + crate_name, + crate_version, + publisher_name.unwrap_or("(unknown username)"), + publisher_email, + chrono::Utc::now().to_rfc2822(), + domain = crate::config::domain_name() + ); + + self.send(emails, &subject, &body) } /// This is supposed to be used only during tests, to retrieve the messages stored in the @@ -96,9 +134,19 @@ or go to https://{domain}/me/pending-invites to manage all of your crate ownersh } } - fn send(&self, recipient: &str, subject: &str, body: &str) -> AppResult<()> { - let email = Message::builder() - .to(recipient.parse()?) + fn send(&self, recipients: &[&str], subject: &str, body: &str) -> AppResult<()> { + let mut recipients_iter = recipients.iter(); + + let mut builder = Message::builder(); + let to = recipients_iter + .next() + .ok_or_else(|| server_error("Email has no recipients"))?; + builder = builder.to(to.parse()?); + for bcc in recipients_iter { + builder = builder.bcc(bcc.parse()?); + } + + let email = builder .from(self.sender_address().parse()?) .subject(subject) .body(body.to_string())?; @@ -122,7 +170,12 @@ or go to https://{domain}/me/pending-invites to manage all of your crate ownersh .map_err(|_| server_error("Email file could not be generated"))?; } EmailBackend::Memory { mails } => mails.lock().unwrap().push(StoredEmail { - to: recipient.into(), + to: to.to_string(), + bcc: recipients + .iter() + .skip(1) + .map(|address| address.to_string()) + .collect(), subject: subject.into(), body: body.into(), }), @@ -176,6 +229,7 @@ impl std::fmt::Debug for EmailBackend { #[derive(Debug, Clone)] pub struct StoredEmail { pub to: String, + pub bcc: Vec, pub subject: String, pub body: String, } @@ -189,7 +243,7 @@ mod tests { let emails = Emails::new_in_memory(); assert_err!(emails.send( - "String.Format(\"{0}.{1}@live.com\", FirstName, LastName)", + &["String.Format(\"{0}.{1}@live.com\", FirstName, LastName)"], "test", "test", )); @@ -199,6 +253,6 @@ mod tests { fn sending_to_valid_email_succeeds() { let emails = Emails::new_in_memory(); - assert_ok!(emails.send("someone@example.com", "test", "test")); + assert_ok!(emails.send(&["someone@example.com"], "test", "test")); } } diff --git a/src/git.rs b/src/git.rs index b6308cf20cd..31c8262a7ba 100644 --- a/src/git.rs +++ b/src/git.rs @@ -266,7 +266,13 @@ impl Repository { } #[swirl::background_job] -pub fn add_crate(env: &Environment, krate: Crate) -> Result<(), PerformError> { +pub fn add_crate( + env: &Environment, + krate: Crate, + owners_emails: Vec, + publisher_name: Option, + publisher_email: String, +) -> Result<(), PerformError> { use std::io::prelude::*; let repo = env.lock_index()?; @@ -279,8 +285,19 @@ pub fn add_crate(env: &Environment, krate: Crate) -> Result<(), PerformError> { file.write_all(b"\n")?; let message: String = format!("Updating crate `{}#{}`", krate.name, krate.vers); - - repo.commit_and_push(&message, &repo.relative_index_file(&krate.name)) + repo.commit_and_push(&message, &repo.relative_index_file(&krate.name))?; + + // Notify crate owners of this new version being published + let emails = owners_emails.iter().map(AsRef::as_ref).collect::>(); + let _ = env.emails.notify_owners( + &emails, + &krate.name, + &krate.vers, + publisher_name.as_deref(), + &publisher_email, + ); + + Ok(()) } /// Yanks or unyanks a crate version. This requires finding the index diff --git a/src/models/krate.rs b/src/models/krate.rs index d87d02ae266..eca26e95578 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -319,6 +319,17 @@ impl Crate { Ok(users.chain(teams).collect()) } + pub fn owners_with_notification_email(&self, conn: &PgConnection) -> QueryResult> { + CrateOwner::by_owner_kind(OwnerKind::User) + .filter(crate_owners::crate_id.eq(self.id)) + .filter(crate_owners::email_notifications.eq(true)) + .inner_join(emails::table.on(crate_owners::owner_id.eq(emails::user_id))) + .filter(emails::verified.eq(true)) + .select(emails::email) + .distinct() + .load(conn) + } + pub fn owner_add( &self, app: &App, diff --git a/src/tests/krate/publish.rs b/src/tests/krate/publish.rs index d3cfa8fb3ed..97b4d020f0c 100644 --- a/src/tests/krate/publish.rs +++ b/src/tests/krate/publish.rs @@ -630,7 +630,7 @@ fn new_krate_records_verified_email() { .select(versions_published_by::email) .first(conn) .unwrap(); - assert_eq!(email, "something@example.com"); + assert_eq!(email, "something+foo@example.com"); }); } diff --git a/src/tests/owners.rs b/src/tests/owners.rs index 11cc1e08a06..36250bddc3b 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -83,6 +83,25 @@ impl MockCookieUser { fn list_invitations(&self) -> InvitationListResponse { self.get("/api/v1/me/crate_owner_invitations").good() } + + fn set_email_notifications(&self, krate_id: i32, email_notifications: bool) { + let body = json!([ + { + "id": krate_id, + "email_notifications": email_notifications, + } + ]); + + #[derive(Deserialize)] + struct Empty {} + + let _: Empty = self + .put( + "/api/v1/me/email_notifications", + body.to_string().as_bytes(), + ) + .good(); + } } impl MockAnonymousUser { @@ -510,3 +529,40 @@ fn extract_token_from_invite_email(emails: &Emails) -> String { let after_pos = before_pos + (&body[before_pos..]).find(after_token).unwrap(); body[before_pos..after_pos].to_string() } + +#[test] +fn test_list_owners_with_notification_email() { + let (app, _, owner, owner_token) = TestApp::init().with_token(); + let owner = owner.as_model(); + + let krate_name = "notification_crate"; + let user_name = "notification_user"; + + let new_user = app.db_new_user(user_name); + let krate = app.db(|conn| CrateBuilder::new(krate_name, owner.id).expect_build(conn)); + + // crate author gets notified + let (owners_notification, email) = app.db(|conn| { + let owners_notification = krate.owners_with_notification_email(conn).unwrap(); + let email = owner.verified_email(conn).unwrap().unwrap(); + (owners_notification, email) + }); + assert_eq!(owners_notification, [email.clone()]); + + // crate author and the new crate owner get notified + owner_token.add_named_owner(krate_name, user_name).good(); + new_user.accept_ownership_invitation(&krate.name, krate.id); + + let (owners_notification, new_user_email) = app.db(|conn| { + let new_user_email = new_user.as_model().verified_email(conn).unwrap().unwrap(); + let owners_notification = krate.owners_with_notification_email(conn).unwrap(); + (owners_notification, new_user_email) + }); + assert_eq!(owners_notification, [email.clone(), new_user_email]); + + // crate owners who disabled notifications don't get notified + new_user.set_email_notifications(krate.id, false); + + let owners_notification = app.db(|conn| krate.owners_with_notification_email(conn).unwrap()); + assert_eq!(owners_notification, [email]); +} diff --git a/src/tests/user.rs b/src/tests/user.rs index e3536d81731..a8b7564f929 100644 --- a/src/tests/user.rs +++ b/src/tests/user.rs @@ -464,7 +464,7 @@ fn test_email_get_and_put() { let (_app, _anon, user) = TestApp::init().with_user(); let json = user.show_me(); - assert_eq!(json.user.email.unwrap(), "something@example.com"); + assert_eq!(json.user.email.unwrap(), "something+foo@example.com"); user.update_email("mango@mangos.mango"); diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index cabb90f82f4..da815b86f9c 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -100,7 +100,7 @@ impl TestApp { use diesel::prelude::*; let user = self.db(|conn| { - let email = "something@example.com"; + let email = format!("something+{}@example.com", username); let user = crate::new_user(username) .create_or_update(None, &self.0.app.emails, conn) @@ -194,6 +194,7 @@ impl TestAppBuilder { index, app.config.uploader.clone(), app.http_client().clone(), + Emails::new_in_memory(), ); Some(