Skip to content

Commit 238f5a5

Browse files
backend: Implement sending notification email when a new version is published
1 parent 30bf5bd commit 238f5a5

File tree

8 files changed

+146
-20
lines changed

8 files changed

+146
-20
lines changed

src/controllers/krate/publish.rs

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

196196
let hex_cksum = cksum.encode_hex::<String>();
197197

198-
// Register this crate in our local git repo.
198+
// Register this crate in our local git repo and send notification emails
199+
// to owners who haven't opted out them.
199200
let git_crate = git::Crate {
200201
name: name.0,
201202
vers: vers.to_string(),
@@ -205,7 +206,8 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
205206
yanked: Some(false),
206207
links,
207208
};
208-
git::add_crate(git_crate).enqueue(&conn)?;
209+
let emails = krate.owners_with_notification_email(&conn)?;
210+
git::add_crate(git_crate, emails, user.name, verified_email_address).enqueue(&conn)?;
209211

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

src/email.rs

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub fn init_config_vars() -> Option<MailgunConfigVars> {
3030
}
3131

3232
fn build_email(
33-
recipient: &str,
33+
recipients: &[&str],
3434
subject: &str,
3535
body: &str,
3636
mailgun_config: &Option<MailgunConfigVars>,
@@ -40,11 +40,11 @@ fn build_email(
4040
.map(|s| s.smtp_login.as_str())
4141
.unwrap_or("test@localhost");
4242

43-
let email = Message::builder()
44-
.to(recipient.parse()?)
45-
.from(sender.parse()?)
46-
.subject(subject)
47-
.body(body)?;
43+
let mut builder = Message::builder();
44+
for recipient in recipients {
45+
builder = builder.to(recipient.parse()?);
46+
}
47+
let email = builder.from(sender.parse()?).subject(subject).body(body)?;
4848

4949
Ok(email)
5050
}
@@ -79,7 +79,7 @@ https://{}/confirm/{}",
7979
token
8080
);
8181

82-
send_email(email, subject, &body)
82+
send_email(&[email], subject, &body)
8383
}
8484

8585
/// Attempts to send a crate owner invitation email. Swallows all errors.
@@ -99,12 +99,51 @@ or go to https://{domain}/me/pending-invites to manage all of your crate ownersh
9999
domain = crate::config::domain_name()
100100
);
101101

102-
let _ = send_email(email, subject, &body);
102+
let _ = send_email(&[email], subject, &body);
103+
}
104+
105+
/// Attempts to send a new crate version published notification to crate owners. Swallows all errors.
106+
pub fn notify_owners(
107+
emails: &[&str],
108+
crate_name: &str,
109+
crate_version: &str,
110+
publisher_name: Option<&str>,
111+
publisher_email: &str,
112+
) {
113+
let subject = format!(
114+
"Crate {} ({}) published to crates.io",
115+
crate_name, crate_version
116+
);
117+
let body = format!(
118+
"A crate you have publish access to has recently released a new version.
119+
120+
Crate: {} ({})
121+
Published by: {} <{}>
122+
Published at: {}
123+
124+
If this publish is expected, you do not need to take further action.
125+
Only if this publish is unexpected, please take immediate steps to secure your account:
126+
127+
* If you suspect your GitHub account was compromised, change your password
128+
* Revoke your API Token
129+
* Yank the version of the crate reported in this email
130+
* Report this incident to RustSec https://rustsec.org
131+
132+
To stop receiving these messages, update your email notification settings at https://{domain}/me",
133+
crate_name,
134+
crate_version,
135+
publisher_name.unwrap_or("(unknown username)"),
136+
publisher_email,
137+
chrono::Utc::now().to_rfc2822(),
138+
domain = crate::config::domain_name()
139+
);
140+
141+
let _ = send_email(emails, &subject, &body);
103142
}
104143

105-
fn send_email(recipient: &str, subject: &str, body: &str) -> AppResult<()> {
144+
fn send_email(recipients: &[&str], subject: &str, body: &str) -> AppResult<()> {
106145
let mailgun_config = init_config_vars();
107-
let email = build_email(recipient, subject, body, &mailgun_config)?;
146+
let email = build_email(recipients, subject, body, &mailgun_config)?;
108147

109148
match mailgun_config {
110149
Some(mailgun_config) => {
@@ -137,7 +176,7 @@ mod tests {
137176
#[test]
138177
fn sending_to_invalid_email_fails() {
139178
let result = send_email(
140-
"String.Format(\"{0}.{1}@live.com\", FirstName, LastName)",
179+
&["String.Format(\"{0}.{1}@live.com\", FirstName, LastName)"],
141180
"test",
142181
"test",
143182
);
@@ -146,7 +185,7 @@ mod tests {
146185

147186
#[test]
148187
fn sending_to_valid_email_succeeds() {
149-
let result = send_email("[email protected]", "test", "test");
188+
let result = send_email(&["[email protected]"], "test", "test");
150189
assert_ok!(result);
151190
}
152191
}

src/git.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use tempfile::{Builder, TempDir};
88
use url::Url;
99

1010
use crate::background_jobs::Environment;
11+
use crate::email::notify_owners;
1112
use crate::models::{DependencyKind, Version};
1213
use crate::schema::versions;
1314

@@ -266,7 +267,13 @@ impl Repository {
266267
}
267268

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

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

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

286304
/// 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
@@ -431,6 +431,17 @@ impl Crate {
431431
Ok(users.chain(teams).collect())
432432
}
433433

434+
pub fn owners_with_notification_email(&self, conn: &PgConnection) -> QueryResult<Vec<String>> {
435+
CrateOwner::by_owner_kind(OwnerKind::User)
436+
.filter(crate_owners::crate_id.eq(self.id))
437+
.filter(crate_owners::email_notifications.eq(true))
438+
.inner_join(emails::table.on(crate_owners::owner_id.eq(emails::user_id)))
439+
.filter(emails::verified.eq(true))
440+
.select(emails::email)
441+
.distinct()
442+
.load(conn)
443+
}
444+
434445
pub fn owner_add(
435446
&self,
436447
app: &App,

src/tests/krate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ fn new_krate_records_verified_email() {
13231323
.select(versions_published_by::email)
13241324
.first(conn)
13251325
.unwrap();
1326-
assert_eq!(email, "[email protected]");
1326+
assert_eq!(email, "something+foo@example.com");
13271327
});
13281328
}
13291329

src/tests/owners.rs

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

87106
#[test]
@@ -424,3 +443,40 @@ fn highest_gh_id_is_most_recent_account_we_know_of() {
424443
let json = invited_user.list_invitations();
425444
assert_eq!(json.crate_owner_invitations.len(), 1);
426445
}
446+
447+
#[test]
448+
fn test_list_owners_with_notification_email() {
449+
let (app, _, owner, owner_token) = TestApp::init().with_token();
450+
let owner = owner.as_model();
451+
452+
let krate_name = "notification_crate";
453+
let user_name = "notification_user";
454+
455+
let new_user = app.db_new_user(user_name);
456+
let krate = app.db(|conn| CrateBuilder::new(krate_name, owner.id).expect_build(conn));
457+
458+
// crate author gets notified
459+
let (owners_notification, email) = app.db(|conn| {
460+
let owners_notification = krate.owners_with_notification_email(conn).unwrap();
461+
let email = owner.verified_email(conn).unwrap().unwrap();
462+
(owners_notification, email)
463+
});
464+
assert_eq!(owners_notification, [email.clone()]);
465+
466+
// crate author and the new crate owner get notified
467+
owner_token.add_named_owner(krate_name, user_name).good();
468+
new_user.accept_ownership_invitation(&krate.name, krate.id);
469+
470+
let (owners_notification, new_user_email) = app.db(|conn| {
471+
let new_user_email = new_user.as_model().verified_email(conn).unwrap().unwrap();
472+
let owners_notification = krate.owners_with_notification_email(conn).unwrap();
473+
(owners_notification, new_user_email)
474+
});
475+
assert_eq!(owners_notification, [email.clone(), new_user_email]);
476+
477+
// crate owners who disabled notifications don't get notified
478+
new_user.set_email_notifications(krate.id, false);
479+
480+
let owners_notification = app.db(|conn| krate.owners_with_notification_email(conn).unwrap());
481+
assert_eq!(owners_notification, [email]);
482+
}

src/tests/user.rs

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

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

467467
user.update_email("[email protected]");
468468

src/tests/util.rs

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

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

143143
let user = crate::new_user(username)
144144
.create_or_update(None, conn)

0 commit comments

Comments
 (0)