Implement rate limiting for e-mail verifications

src/controllers/user/me.rs

@@ -1,4 +1,6 @@
 use crate::auth::AuthCheck;
+use crate::rate_limiter::LimitedAction;
+use crate::rate_limiter::RateLimiter;
 use secrecy::{ExposeSecret, SecretString};
 use std::collections::HashMap;
 
@@ -159,11 +161,13 @@
             // an invalid email set in their GitHub profile, and we should let them sign in even though
             // we're trying to silently use their invalid address during signup and can't send them an
             // email. They'll then have to provide a valid email address.
-            let email = UserConfirmEmail {
-                user_name: &user.gh_login,
-                domain: &state.emails.domain,
+            let email = UserConfirmEmail::new(
+                &state.rate_limiter,
+                conn,
+                user,
+                &state.emails.domain,
                 token,
-            };
+            )?;
 
             let _ = state.emails.send(user_email, email);
 
@@ -222,11 +226,13 @@
                 .optional()?
                 .ok_or_else(|| bad_request("Email could not be found"))?;
 
-            let email1 = UserConfirmEmail {
-                user_name: &user.gh_login,
-                domain: &state.emails.domain,
-                token: email.token,
-            };
+            let email1 = UserConfirmEmail::new(
+                &state.rate_limiter,
+                conn,
+                user,
+                &state.emails.domain,
+                email.token,
+            )?;
 
             state.emails.send(&email.email, email1).map_err(Into::into)
         })?;
@@ -300,9 +306,26 @@
 }
 
 pub struct UserConfirmEmail<'a> {
-    pub user_name: &'a str,
-    pub domain: &'a str,
-    pub token: SecretString,
+    user_name: &'a str,
+    domain: &'a str,
+    token: SecretString,
+}
+
+impl<'a> UserConfirmEmail<'a> {
+    pub fn new(
+        rate_limiter: &RateLimiter,
+        conn: &mut PgConnection,
+        user: &'a User,
+        domain: &'a str,
+        token: SecretString,
+    ) -> AppResult<Self> {
+        rate_limiter.check_rate_limit(user.id, LimitedAction::VerifyEmail, conn)?;
+        Ok(Self {
+            user_name: &user.gh_login,
+            domain,
+            token,
+        })
+    }
 }
 
 impl crate::email::Email for UserConfirmEmail<'_> {

src/controllers/user/session.rs

@@ -1,4 +1,5 @@
 use crate::controllers::frontend_prelude::*;
+use crate::rate_limiter::RateLimiter;
 
 use axum::extract::{FromRequestParts, Query};
 use oauth2::reqwest::http_client;
@@ -108,8 +109,13 @@
 
         // Fetch the user info from GitHub using the access token we just got and create a user record
         let ghuser = Handle::current().block_on(app.github.current_user(token))?;
-        let user =
-            save_user_to_database(&ghuser, token.secret(), &app.emails, &mut *app.db_write()?)?;
+        let user = save_user_to_database(
+            &ghuser,
+            token.secret(),
+            &app.emails,
+            &app.rate_limiter,
+            &mut *app.db_write()?,
+        )?;
 
         // Log in by setting a cookie and the middleware authentication
         session.insert("user_id".to_string(), user.id.to_string());
@@ -125,6 +131,7 @@
     user: &GithubUser,
     access_token: &str,
     emails: &Emails,
+    rate_limiter: &RateLimiter,
     conn: &mut PgConnection,
 ) -> AppResult<User> {
     NewUser::new(
@@ -134,7 +141,7 @@
         user.avatar_url.as_deref(),
         access_token,
     )
-    .create_or_update(user.email.as_deref(), emails, conn)
+    .create_or_update(user.email.as_deref(), emails, rate_limiter, conn)
     .map_err(Into::into)
     .or_else(|e: BoxedAppError| {
         // If we're in read only mode, we can't update their details
@@ -165,6 +172,7 @@
     #[test]
     fn gh_user_with_invalid_email_doesnt_fail() {
         let emails = Emails::new_in_memory();
+        let rate_limiter = RateLimiter::new(Default::default());
         let (_test_db, conn) = &mut test_db_connection();
         let gh_user = GithubUser {
             email: Some("String.Format(\"{0}.{1}@live.com\", FirstName, LastName)".into()),
@@ -173,7 +181,8 @@
             id: -1,
             avatar_url: None,
         };
-        let result = save_user_to_database(&gh_user, "arbitrary_token", &emails, conn);
+        let result =
+            save_user_to_database(&gh_user, "arbitrary_token", &emails, &rate_limiter, conn);
 
         assert!(
             result.is_ok(),

src/models/user.rs

@@ -5,6 +5,7 @@ use secrecy::SecretString;
 use crate::app::App;
 use crate::controllers::user::me::UserConfirmEmail;
 use crate::email::Emails;
+use crate::rate_limiter::RateLimiter;
 use crate::util::errors::AppResult;
 
 use crate::models::{ApiToken, Crate, CrateOwner, Email, NewEmail, Owner, OwnerKind, Rights};
@@ -58,8 +59,9 @@ impl<'a> NewUser<'a> {
         &self,
         email: Option<&'a str>,
         emails: &Emails,
+        rate_limiter: &RateLimiter,
         conn: &mut PgConnection,
-    ) -> QueryResult<User> {
+    ) -> AppResult<User> {
         use diesel::dsl::sql;
         use diesel::insert_into;
         use diesel::pg::upsert::excluded;
@@ -102,12 +104,10 @@ impl<'a> NewUser<'a> {
                     .map(SecretString::new);
 
                 if let Some(token) = token {
+                    let email =
+                        UserConfirmEmail::new(rate_limiter, conn, &user, &emails.domain, token)?;
+
                     // Swallows any error. Some users might insert an invalid email address here.
-                    let email = UserConfirmEmail {
-                        user_name: &user.gh_login,
-                        domain: &emails.domain,
-                        token,
-                    };
                     let _ = emails.send(user_email, email);
                 }
             }

src/rate_limiter.rs

@@ -14,15 +14,17 @@
         PublishNew = 0,
         PublishUpdate = 1,
         YankUnyank = 2,
+        VerifyEmail = 3,
     }
 }
 
 impl LimitedAction {
     pub fn default_rate_seconds(&self) -> u64 {
         match self {
-            LimitedAction::PublishNew => 10 * 60, // 10 minutes
-            LimitedAction::PublishUpdate => 60,   // 1 minute
-            LimitedAction::YankUnyank => 60,      // 1 minute
+            LimitedAction::PublishNew => 10 * 60,  // 10 minutes
+            LimitedAction::PublishUpdate => 60,    // 1 minute
+            LimitedAction::YankUnyank => 60,       // 1 minute
+            LimitedAction::VerifyEmail => 30 * 60, // 30 minutes
         }
     }
 
@@ -31,6 +33,7 @@
             LimitedAction::PublishNew => 5,
             LimitedAction::PublishUpdate => 30,
             LimitedAction::YankUnyank => 100,
+            LimitedAction::VerifyEmail => 3,
         }
     }
 
@@ -39,6 +42,7 @@
             LimitedAction::PublishNew => "PUBLISH_NEW",
             LimitedAction::PublishUpdate => "PUBLISH_UPDATE",
             LimitedAction::YankUnyank => "YANK_UNYANK",
+            LimitedAction::VerifyEmail => "VERIFY_EMAIL",
         }
     }
 
@@ -53,6 +57,9 @@
             LimitedAction::YankUnyank => {
                 "You have yanked or unyanked too many versions in a short period of time"
             }
+            LimitedAction::VerifyEmail => {
+                "You have attempted to verify too many e-mail addresses in a short period of time"
+            }
         }
     }
 }
@@ -182,7 +189,7 @@
     use crate::test_util::*;
 
     #[test]
-    fn default_rate_limits() -> QueryResult<()> {
+    fn default_rate_limits() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -246,11 +253,26 @@
         }
         assert_eq!(expected_last_refill_times, last_refill_times);
 
+        // E-mail verification has a burst of 3 and a refill time of 30 minutes, which means we
+        // should be able to verify an e-mail address every 30 minutes, always have tokens
+        // remaining, and set the last_refill based on the refill time.
+        let action = LimitedAction::VerifyEmail;
+        let mut last_refill_times = vec![];
+        let mut expected_last_refill_times = vec![];
+        for publish_num in 1..=3 {
+            let publish_time = now + chrono::Duration::minutes(30 * publish_num);
+            let bucket = rate.take_token(user_id, action, publish_time, conn)?;
+
+            last_refill_times.push(bucket.last_refill);
+            expected_last_refill_times.push(publish_time);
+        }
+        assert_eq!(expected_last_refill_times, last_refill_times);
+
         Ok(())
     }
 
     #[test]
-    fn take_token_with_no_bucket_creates_new_one() -> QueryResult<()> {
+    fn take_token_with_no_bucket_creates_new_one() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -297,7 +319,7 @@
     }
 
     #[test]
-    fn take_token_with_existing_bucket_modifies_existing_bucket() -> QueryResult<()> {
+    fn take_token_with_existing_bucket_modifies_existing_bucket() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -320,7 +342,7 @@
     }
 
     #[test]
-    fn take_token_after_delay_refills() -> QueryResult<()> {
+    fn take_token_after_delay_refills() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -344,7 +366,7 @@
     }
 
     #[test]
-    fn refill_subsecond_rate() -> QueryResult<()> {
+    fn refill_subsecond_rate() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         // Subsecond rates have floating point rounding issues, so use a known
         // timestamp that rounds fine
@@ -372,7 +394,7 @@
     }
 
     #[test]
-    fn last_refill_always_advanced_by_multiple_of_rate() -> QueryResult<()> {
+    fn last_refill_always_advanced_by_multiple_of_rate() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -401,7 +423,7 @@
     }
 
     #[test]
-    fn zero_tokens_returned_when_user_has_no_tokens_left() -> QueryResult<()> {
+    fn zero_tokens_returned_when_user_has_no_tokens_left() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -427,7 +449,7 @@
     }
 
     #[test]
-    fn a_user_with_no_tokens_gets_a_token_after_exactly_rate() -> QueryResult<()> {
+    fn a_user_with_no_tokens_gets_a_token_after_exactly_rate() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -452,7 +474,7 @@
     }
 
     #[test]
-    fn tokens_never_refill_past_burst() -> QueryResult<()> {
+    fn tokens_never_refill_past_burst() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -477,7 +499,7 @@
     }
 
     #[test]
-    fn two_actions_dont_interfere_with_each_other() -> QueryResult<()> {
+    fn two_actions_dont_interfere_with_each_other() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -520,7 +542,7 @@
     }
 
     #[test]
-    fn override_is_used_instead_of_global_burst_if_present() -> QueryResult<()> {
+    fn override_is_used_instead_of_global_burst_if_present() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -550,7 +572,7 @@
     }
 
     #[test]
-    fn overrides_can_expire() -> QueryResult<()> {
+    fn overrides_can_expire() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
 
@@ -597,7 +619,7 @@
     }
 
     #[test]
-    fn override_is_different_for_each_action() -> QueryResult<()> {
+    fn override_is_different_for_each_action() -> AppResult<()> {
         let (_test_db, conn) = &mut test_db_connection();
         let now = now();
         let user_id = new_user(conn, "user")?;
@@ -636,30 +658,35 @@
         Ok(())
     }
 
-    fn new_user(conn: &mut PgConnection, gh_login: &str) -> QueryResult<i32> {
+    fn new_user(conn: &mut PgConnection, gh_login: &str) -> AppResult<i32> {
         use crate::models::NewUser;
 
         let user = NewUser {
             gh_login,
             ..NewUser::default()
         }
-        .create_or_update(None, &Emails::new_in_memory(), conn)?;
+        .create_or_update(
+            None,
+            &Emails::new_in_memory(),
+            &RateLimiter::new(Default::default()),
+            conn,
+        )?;
         Ok(user.id)
     }
 
     fn new_user_bucket(
         conn: &mut PgConnection,
         tokens: i32,
         now: NaiveDateTime,
-    ) -> QueryResult<Bucket> {
-        diesel::insert_into(publish_limit_buckets::table)
+    ) -> AppResult<Bucket> {
+        Ok(diesel::insert_into(publish_limit_buckets::table)
             .values(Bucket {
                 user_id: new_user(conn, "new_user")?,
                 tokens,
                 last_refill: now,
                 action: LimitedAction::PublishNew,
             })
-            .get_result(conn)
+            .get_result(conn)?)
     }
 
     struct SampleRateLimiter {