Merge pull request #11420 from Turbo87/jinja-emails

Use `minijinja` templates for emails

Author: Turbo87

src/controllers/github/secret_scanning.rs

@@ -1,5 +1,5 @@
 use crate::app::AppState;
-use crate::email::Email;
+use crate::email::EmailMessage;
 use crate::models::{ApiToken, User};
 use crate::schema::{api_tokens, crate_owners, crates, emails};
 use crate::util::errors::{AppResult, BoxedAppError, bad_request};
@@ -16,6 +16,7 @@ use diesel::prelude::*;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
 use futures_util::TryStreamExt;
 use http::HeaderMap;
+use minijinja::context;
 use p256::PublicKey;
 use p256::ecdsa::VerifyingKey;
 use p256::ecdsa::signature::Verifier;
@@ -25,6 +26,7 @@ use std::str::FromStr;
 use std::sync::LazyLock;
 use std::time::Duration;
 use tokio::sync::Mutex;
+use tracing::warn;
 
 // Minimum number of seconds to wait before refreshing cache of GitHub's public keys
 const PUBLIC_KEY_CACHE_LIFETIME: Duration = Duration::from_secs(60 * 60 * 24); // 24 hours
@@ -226,13 +228,16 @@ async fn send_notification_email(
         return Err(anyhow!("No address found"));
     };
 
-    let email = TokenExposedEmail {
-        domain: &state.config.domain_name,
-        reporter: "GitHub",
-        source: &alert.source,
-        token_name: &token.name,
-        url: &alert.url,
-    };
+    let email = EmailMessage::from_template(
+        "token_exposed",
+        context! {
+            domain => state.config.domain_name,
+            reporter => "GitHub",
+            source => alert.source,
+            token_name => token.name,
+            url => if alert.url.is_empty() { "" } else { &alert.url }
+        },
+    )?;
 
     state.emails.send(&recipient, email).await?;
 
@@ -285,12 +290,24 @@ async fn send_trustpub_notification_emails(
 
     // Send notifications in sorted order by email for consistent testing
     for (email, crate_names) in notifications {
-        let email_template = TrustedPublishingTokenExposedEmail {
-            domain: &state.config.domain_name,
-            reporter: "GitHub",
-            source: &alert.source,
-            crate_names: &crate_names.iter().cloned().collect::<Vec<_>>(),
-            url: &alert.url,
+        let message = EmailMessage::from_template(
+            "trustpub_token_exposed",
+            context! {
+                domain => state.config.domain_name,
+                reporter => "GitHub",
+                source => alert.source,
+                crate_names,
+                url => alert.url
+            },
+        );
+
+        let Ok(email_template) = message.inspect_err(|error| {
+            warn!(
+                %email, ?crate_names, ?error,
+                "Failed to create trusted publishing token exposure email template"
+            );
+        }) else {
+            continue;
         };
 
         if let Err(error) = state.emails.send(&email, email_template).await {
@@ -304,104 +321,6 @@ async fn send_trustpub_notification_emails(
     Ok(())
 }
 
-struct TokenExposedEmail<'a> {
-    domain: &'a str,
-    reporter: &'a str,
-    source: &'a str,
-    token_name: &'a str,
-    url: &'a str,
-}
-
-impl Email for TokenExposedEmail<'_> {
-    fn subject(&self) -> String {
-        format!(
-            "crates.io: Your API token \"{}\" has been revoked",
-            self.token_name
-        )
-    }
-
-    fn body(&self) -> String {
-        let mut body = format!(
-            "{reporter} has notified us that your crates.io API token {token_name} \
-has been exposed publicly. We have revoked this token as a precaution.
-
-Please review your account at https://{domain} to confirm that no \
-unexpected changes have been made to your settings or crates.
-
-Source type: {source}",
-            domain = self.domain,
-            reporter = self.reporter,
-            source = self.source,
-            token_name = self.token_name,
-        );
-        if self.url.is_empty() {
-            body.push_str("\n\nWe were not informed of the URL where the token was found.");
-        } else {
-            body.push_str(&format!("\n\nURL where the token was found: {}", self.url));
-        }
-
-        body
-    }
-}
-
-struct TrustedPublishingTokenExposedEmail<'a> {
-    domain: &'a str,
-    reporter: &'a str,
-    source: &'a str,
-    crate_names: &'a [String],
-    url: &'a str,
-}
-
-impl Email for TrustedPublishingTokenExposedEmail<'_> {
-    fn subject(&self) -> String {
-        "crates.io: Your Trusted Publishing token has been revoked".to_string()
-    }
-
-    fn body(&self) -> String {
-        let authorization = if self.crate_names.len() == 1 {
-            format!(
-                "This token was only authorized to publish the \"{}\" crate.",
-                self.crate_names[0]
-            )
-        } else {
-            format!(
-                "This token was authorized to publish the following crates: \"{}\".",
-                self.crate_names.join("\", \"")
-            )
-        };
-
-        let mut body = format!(
-            "{reporter} has notified us that one of your crates.io Trusted Publishing tokens \
-has been exposed publicly. We have revoked this token as a precaution.
-
-{authorization}
-
-Please review your account at https://{domain} and your GitHub repository \
-settings to confirm that no unexpected changes have been made to your crates \
-or trusted publishing configurations.
-
-Source type: {source}",
-            domain = self.domain,
-            reporter = self.reporter,
-            source = self.source,
-        );
-
-        if self.url.is_empty() {
-            body.push_str("\n\nWe were not informed of the URL where the token was found.");
-        } else {
-            body.push_str(&format!("\n\nURL where the token was found: {}", self.url));
-        }
-
-        body.push_str(
-            "\n\nTrusted Publishing tokens are temporary and used for automated \
-publishing from GitHub Actions. If this exposure was unexpected, please review \
-your repository's workflow files and secrets.",
-        );
-
-        body
-    }
-}
-
 #[derive(Deserialize, Serialize)]
 pub struct GitHubSecretAlertFeedback {
     pub token_raw: String,

src/controllers/krate/delete.rs

@@ -2,7 +2,7 @@ use crate::app::AppState;
 use crate::auth::AuthCheck;
 use crate::controllers::helpers::authorization::Rights;
 use crate::controllers::krate::CratePath;
-use crate::email::Email;
+use crate::email::EmailMessage;
 use crate::models::NewDeletedCrate;
 use crate::schema::{crate_downloads, crates, dependencies};
 use crate::util::errors::{AppResult, BoxedAppError, custom};
@@ -18,6 +18,8 @@ use diesel_async::scoped_futures::ScopedFutureExt;
 use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
 use http::StatusCode;
 use http::request::Parts;
+use minijinja::context;
+use tracing::error;
 
 const DOWNLOADS_PER_MONTH_LIMIT: u64 = 500;
 const AVAILABLE_AFTER: TimeDelta = TimeDelta::hours(24);
@@ -147,10 +149,13 @@ pub async fn delete_crate(
 
     let email_future = async {
         if let Some(recipient) = user.email(&mut conn).await? {
-            let email = CrateDeletionEmail {
-                user: &user.gh_login,
-                krate: &crate_name,
-            };
+            let email = EmailMessage::from_template(
+                "crate_deletion",
+                context! {
+                    user => user.gh_login,
+                    krate => crate_name
+                },
+            )?;
 
             app.emails.send(&recipient, email).await?
         }
@@ -193,33 +198,6 @@ async fn has_rev_dep(conn: &mut AsyncPgConnection, crate_id: i32) -> QueryResult
     Ok(rev_dep.is_some())
 }
 
-/// Email template for notifying a crate owner about a crate being deleted.
-///
-/// The owner usually should be aware of the deletion since they initiated it,
-/// but this email can be helpful in detecting malicious account activity.
-#[derive(Debug, Clone)]
-struct CrateDeletionEmail<'a> {
-    user: &'a str,
-    krate: &'a str,
-}
-
-impl Email for CrateDeletionEmail<'_> {
-    fn subject(&self) -> String {
-        format!("crates.io: Deleted \"{}\" crate", self.krate)
-    }
-
-    fn body(&self) -> String {
-        format!(
-            "Hi {},
-
-Your \"{}\" crate has been deleted, per your request.
-
-If you did not initiate this deletion, your account may have been compromised. Please contact us at help@crates.io.",
-            self.user, self.krate
-        )
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/controllers/krate/owners.rs

@@ -11,7 +11,7 @@ use crate::models::{
 use crate::util::errors::{AppResult, BoxedAppError, bad_request, crate_not_found, custom};
 use crate::views::EncodableOwner;
 use crate::{App, app::AppState};
-use crate::{auth::AuthCheck, email::Email};
+use crate::{auth::AuthCheck, email::EmailMessage};
 use axum::Json;
 use chrono::Utc;
 use crates_io_github::{GitHubClient, GitHubError};
@@ -20,9 +20,11 @@ use diesel_async::scoped_futures::ScopedFutureExt;
 use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
 use http::StatusCode;
 use http::request::Parts;
+use minijinja::context;
 use oauth2::AccessToken;
-use secrecy::{ExposeSecret, SecretString};
+use secrecy::ExposeSecret;
 use thiserror::Error;
+use tracing::warn;
 
 #[derive(Debug, Serialize, utoipa::ToSchema)]
 pub struct UsersResponse {
@@ -240,13 +242,20 @@ async fn modify_owners(
                                 if let Some(recipient) =
                                     invitee.verified_email(conn).await.ok().flatten()
                                 {
-                                    emails.push(OwnerInviteEmail {
-                                        recipient_email_address: recipient,
-                                        inviter: user.gh_login.clone(),
-                                        domain: app.emails.domain.clone(),
-                                        crate_name: krate.name.clone(),
-                                        token,
-                                    });
+                                    let email = EmailMessage::from_template(
+                                        "owner_invite",
+                                        context! {
+                                            inviter => user.gh_login,
+                                            domain => app.emails.domain,
+                                            crate_name => krate.name,
+                                            token => token.expose_secret()
+                                        },
+                                    );
+
+                                    match email {
+                                        Ok(email_msg) => emails.push((recipient, email_msg)),
+                                        Err(error) => warn!("Failed to render owner invite email template: {error}"),
+                                    }
                                 }
                             }
 
@@ -291,11 +300,9 @@ async fn modify_owners(
 
     // Send the accumulated invite emails now the database state has
     // committed.
-    for email in emails {
-        let addr = email.recipient_email_address().to_string();
-
-        if let Err(e) = app.emails.send(&addr, email).await {
-            warn!("Failed to send co-owner invite email: {e}");
+    for (recipient, email) in emails {
+        if let Err(error) = app.emails.send(&recipient, email).await {
+            warn!("Failed to send owner invite email to {recipient}: {error}");
         }
     }
 
@@ -503,41 +510,3 @@ impl From<OwnerRemoveError> for BoxedAppError {
         }
     }
 }
-
-pub struct OwnerInviteEmail {
-    /// The destination email address for this email.
-    recipient_email_address: String,
-
-    /// Email body variables.
-    inviter: String,
-    domain: String,
-    crate_name: String,
-    token: SecretString,
-}
-
-impl OwnerInviteEmail {
-    pub fn recipient_email_address(&self) -> &str {
-        &self.recipient_email_address
-    }
-}
-
-impl Email for OwnerInviteEmail {
-    fn subject(&self) -> String {
-        format!(
-            "crates.io: Ownership invitation for \"{}\"",
-            self.crate_name
-        )
-    }
-
-    fn body(&self) -> String {
-        format!(
-            "{user_name} has invited you to become an owner of the crate {crate_name}!\n
-Visit https://{domain}/accept-invite/{token} to accept this invitation,
-or go to https://{domain}/me/pending-invites to manage all of your crate ownership invitations.",
-            user_name = self.inviter,
-            domain = self.domain,
-            crate_name = self.crate_name,
-            token = self.token.expose_secret(),
-        )
-    }
-}