Skip to content

Commit 5529141

Browse files
backend: Implement sending notification email when a new version is published
1 parent e5757df commit 5529141

File tree

9 files changed

+163
-18
lines changed

9 files changed

+163
-18
lines changed

src/background_jobs.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ use diesel::r2d2::PoolError;
66
use swirl::PerformError;
77

88
use crate::db::{DieselPool, DieselPooledConn};
9+
use crate::email::Emails;
910
use crate::git::Repository;
1011
use crate::uploaders::Uploader;
11-
use crate::email::Emails;
1212

1313
impl<'a> swirl::db::BorrowedConnection<'a> for DieselPool {
1414
type Connection = DieselPooledConn<'a>;
@@ -43,7 +43,12 @@ impl Clone for Environment {
4343

4444
impl Environment {
4545
pub fn new(index: Repository, uploader: Uploader, http_client: Client, emails: Emails) -> Self {
46-
Self::new_shared(Arc::new(Mutex::new(index)), uploader, http_client, Arc::new(emails))
46+
Self::new_shared(
47+
Arc::new(Mutex::new(index)),
48+
uploader,
49+
http_client,
50+
Arc::new(emails),
51+
)
4752
}
4853

4954
pub fn new_shared(

src/controllers/krate/publish.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
204204

205205
let hex_cksum = cksum.encode_hex::<String>();
206206

207-
// Register this crate in our local git repo.
207+
// Register this crate in our local git repo and send notification emails
208+
// to owners who haven't opted out of them.
208209
let git_crate = git::Crate {
209210
name: name.0,
210211
vers: vers.to_string(),
@@ -214,7 +215,8 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
214215
yanked: Some(false),
215216
links,
216217
};
217-
git::add_crate(git_crate).enqueue(&conn)?;
218+
let emails = krate.owners_with_notification_email(&conn)?;
219+
git::add_crate(git_crate, emails, user.name, verified_email_address).enqueue(&conn)?;
218220

219221
// The `other` field on `PublishWarnings` was introduced to handle a temporary warning
220222
// that is no longer needed. As such, crates.io currently does not return any `other`

src/email.rs

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ https://{}/confirm/{}",
6161
token
6262
);
6363

64-
self.send(email, subject, &body)
64+
self.send(&[email], subject, &body)
6565
}
6666

6767
/// 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
8383
domain = crate::config::domain_name()
8484
);
8585

86-
self.send(email, subject, &body)
86+
self.send(&[email], subject, &body)
87+
}
88+
89+
/// Attempts to send a new crate version published notification to crate owners.
90+
pub fn notify_owners(
91+
&self,
92+
emails: &[&str],
93+
crate_name: &str,
94+
crate_version: &str,
95+
publisher_name: Option<&str>,
96+
publisher_email: &str,
97+
) -> AppResult<()> {
98+
let subject = format!(
99+
"Crate {} ({}) published to {}",
100+
crate_name,
101+
crate_version,
102+
crate::config::domain_name()
103+
);
104+
let body = format!(
105+
"A crate you have publish access to has recently released a new version.
106+
Crate: {} ({})
107+
Published by: {} <{}>
108+
Published at: {}
109+
If this publish is expected, you do not need to take further action.
110+
Only if this publish is unexpected, please take immediate steps to secure your account:
111+
* If you suspect your GitHub account was compromised, change your password
112+
* Revoke your API Token
113+
* Yank the version of the crate reported in this email
114+
* Report this incident to RustSec https://rustsec.org
115+
To stop receiving these messages, update your email notification settings at https://{domain}/me",
116+
crate_name,
117+
crate_version,
118+
publisher_name.unwrap_or("(unknown username)"),
119+
publisher_email,
120+
chrono::Utc::now().to_rfc2822(),
121+
domain = crate::config::domain_name()
122+
);
123+
124+
self.send(emails, &subject, &body)
87125
}
88126

89127
/// 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
96134
}
97135
}
98136

99-
fn send(&self, recipient: &str, subject: &str, body: &str) -> AppResult<()> {
100-
let email = Message::builder()
101-
.to(recipient.parse()?)
137+
fn send(&self, recipients: &[&str], subject: &str, body: &str) -> AppResult<()> {
138+
let mut recipients_iter = recipients.iter();
139+
140+
let mut builder = Message::builder();
141+
let to = recipients_iter
142+
.next()
143+
.ok_or_else(|| server_error("Email has no recipients"))?;
144+
builder = builder.to(to.parse()?);
145+
for bcc in recipients_iter {
146+
builder = builder.bcc(bcc.parse()?);
147+
}
148+
149+
let email = builder
102150
.from(self.sender_address().parse()?)
103151
.subject(subject)
104152
.body(body.to_string())?;
@@ -122,7 +170,12 @@ or go to https://{domain}/me/pending-invites to manage all of your crate ownersh
122170
.map_err(|_| server_error("Email file could not be generated"))?;
123171
}
124172
EmailBackend::Memory { mails } => mails.lock().unwrap().push(StoredEmail {
125-
to: recipient.into(),
173+
to: to.to_string(),
174+
bcc: recipients
175+
.iter()
176+
.skip(1)
177+
.map(|address| address.to_string())
178+
.collect(),
126179
subject: subject.into(),
127180
body: body.into(),
128181
}),
@@ -176,6 +229,7 @@ impl std::fmt::Debug for EmailBackend {
176229
#[derive(Debug, Clone)]
177230
pub struct StoredEmail {
178231
pub to: String,
232+
pub bcc: Vec<String>,
179233
pub subject: String,
180234
pub body: String,
181235
}
@@ -189,7 +243,7 @@ mod tests {
189243
let emails = Emails::new_in_memory();
190244

191245
assert_err!(emails.send(
192-
"String.Format(\"{0}.{1}@live.com\", FirstName, LastName)",
246+
&["String.Format(\"{0}.{1}@live.com\", FirstName, LastName)"],
193247
"test",
194248
"test",
195249
));
@@ -199,6 +253,6 @@ mod tests {
199253
fn sending_to_valid_email_succeeds() {
200254
let emails = Emails::new_in_memory();
201255

202-
assert_ok!(emails.send("[email protected]", "test", "test"));
256+
assert_ok!(emails.send(&["[email protected]"], "test", "test"));
203257
}
204258
}

src/git.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,13 @@ impl Repository {
266266
}
267267

268268
#[swirl::background_job]
269-
pub fn add_crate(env: &Environment, krate: Crate) -> Result<(), PerformError> {
269+
pub fn add_crate(
270+
env: &Environment,
271+
krate: Crate,
272+
owners_emails: Vec<String>,
273+
publisher_name: Option<String>,
274+
publisher_email: String,
275+
) -> Result<(), PerformError> {
270276
use std::io::prelude::*;
271277

272278
let repo = env.lock_index()?;
@@ -279,8 +285,19 @@ pub fn add_crate(env: &Environment, krate: Crate) -> Result<(), PerformError> {
279285
file.write_all(b"\n")?;
280286

281287
let message: String = format!("Updating crate `{}#{}`", krate.name, krate.vers);
282-
283-
repo.commit_and_push(&message, &repo.relative_index_file(&krate.name))
288+
repo.commit_and_push(&message, &repo.relative_index_file(&krate.name))?;
289+
290+
// Notify crate owners of this new version being published
291+
let emails = owners_emails.iter().map(AsRef::as_ref).collect::<Vec<_>>();
292+
let _ = env.emails.notify_owners(
293+
&emails,
294+
&krate.name,
295+
&krate.vers,
296+
publisher_name.as_deref(),
297+
&publisher_email,
298+
);
299+
300+
Ok(())
284301
}
285302

286303
/// Yanks or unyanks a crate version. This requires finding the index

src/models/krate.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,17 @@ impl Crate {
319319
Ok(users.chain(teams).collect())
320320
}
321321

322+
pub fn owners_with_notification_email(&self, conn: &PgConnection) -> QueryResult<Vec<String>> {
323+
CrateOwner::by_owner_kind(OwnerKind::User)
324+
.filter(crate_owners::crate_id.eq(self.id))
325+
.filter(crate_owners::email_notifications.eq(true))
326+
.inner_join(emails::table.on(crate_owners::owner_id.eq(emails::user_id)))
327+
.filter(emails::verified.eq(true))
328+
.select(emails::email)
329+
.distinct()
330+
.load(conn)
331+
}
332+
322333
pub fn owner_add(
323334
&self,
324335
app: &App,

src/tests/krate/publish.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ fn new_krate_records_verified_email() {
630630
.select(versions_published_by::email)
631631
.first(conn)
632632
.unwrap();
633-
assert_eq!(email, "[email protected]");
633+
assert_eq!(email, "something+foo@example.com");
634634
});
635635
}
636636

src/tests/owners.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,25 @@ impl MockCookieUser {
8383
fn list_invitations(&self) -> InvitationListResponse {
8484
self.get("/api/v1/me/crate_owner_invitations").good()
8585
}
86+
87+
fn set_email_notifications(&self, krate_id: i32, email_notifications: bool) {
88+
let body = json!([
89+
{
90+
"id": krate_id,
91+
"email_notifications": email_notifications,
92+
}
93+
]);
94+
95+
#[derive(Deserialize)]
96+
struct Empty {}
97+
98+
let _: Empty = self
99+
.put(
100+
"/api/v1/me/email_notifications",
101+
body.to_string().as_bytes(),
102+
)
103+
.good();
104+
}
86105
}
87106

88107
impl MockAnonymousUser {
@@ -510,3 +529,40 @@ fn extract_token_from_invite_email(emails: &Emails) -> String {
510529
let after_pos = before_pos + (&body[before_pos..]).find(after_token).unwrap();
511530
body[before_pos..after_pos].to_string()
512531
}
532+
533+
#[test]
534+
fn test_list_owners_with_notification_email() {
535+
let (app, _, owner, owner_token) = TestApp::init().with_token();
536+
let owner = owner.as_model();
537+
538+
let krate_name = "notification_crate";
539+
let user_name = "notification_user";
540+
541+
let new_user = app.db_new_user(user_name);
542+
let krate = app.db(|conn| CrateBuilder::new(krate_name, owner.id).expect_build(conn));
543+
544+
// crate author gets notified
545+
let (owners_notification, email) = app.db(|conn| {
546+
let owners_notification = krate.owners_with_notification_email(conn).unwrap();
547+
let email = owner.verified_email(conn).unwrap().unwrap();
548+
(owners_notification, email)
549+
});
550+
assert_eq!(owners_notification, [email.clone()]);
551+
552+
// crate author and the new crate owner get notified
553+
owner_token.add_named_owner(krate_name, user_name).good();
554+
new_user.accept_ownership_invitation(&krate.name, krate.id);
555+
556+
let (owners_notification, new_user_email) = app.db(|conn| {
557+
let new_user_email = new_user.as_model().verified_email(conn).unwrap().unwrap();
558+
let owners_notification = krate.owners_with_notification_email(conn).unwrap();
559+
(owners_notification, new_user_email)
560+
});
561+
assert_eq!(owners_notification, [email.clone(), new_user_email]);
562+
563+
// crate owners who disabled notifications don't get notified
564+
new_user.set_email_notifications(krate.id, false);
565+
566+
let owners_notification = app.db(|conn| krate.owners_with_notification_email(conn).unwrap());
567+
assert_eq!(owners_notification, [email]);
568+
}

src/tests/user.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ fn test_email_get_and_put() {
464464
let (_app, _anon, user) = TestApp::init().with_user();
465465

466466
let json = user.show_me();
467-
assert_eq!(json.user.email.unwrap(), "[email protected]");
467+
assert_eq!(json.user.email.unwrap(), "something+foo@example.com");
468468

469469
user.update_email("[email protected]");
470470

src/tests/util/test_app.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ impl TestApp {
100100
use diesel::prelude::*;
101101

102102
let user = self.db(|conn| {
103-
let email = "[email protected]";
103+
let email = format!("something+{}@example.com", username);
104104

105105
let user = crate::new_user(username)
106106
.create_or_update(None, &self.0.app.emails, conn)

0 commit comments

Comments
 (0)