From edf516efc80c059f26de63a02a19cdae39aaa566 Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Sat, 26 Jul 2025 12:38:07 +0100 Subject: [PATCH 1/7] Allow `emails` table to contain multiple emails per user --- crates/crates_io_database/src/models/email.rs | 76 ++-- crates/crates_io_database/src/models/user.rs | 6 +- crates/crates_io_database/src/schema.patch | 16 + crates/crates_io_database/src/schema.rs | 3 + .../tests/email_constraints.rs | 333 ++++++++++++++++++ ...ail_constraints__create_primary_email.snap | 12 + ..._create_primary_email_when_one_exists.snap | 20 ++ ...create_same_email_for_different_users.snap | 22 ++ ...l_constraints__create_secondary_email.snap | 22 ++ ...dary_email_with_same_email_as_primary.snap | 18 + ...reate_secondary_email_without_primary.snap | 10 + ...l_constraints__create_too_many_emails.snap | 10 + .../email_constraints__delete_only_email.snap | 7 + ...__delete_primary_email_with_secondary.snap | 10 + ...l_constraints__delete_secondary_email.snap | 7 + ...s__demote_primary_without_new_primary.snap | 10 + ...traints__promote_secondary_to_primary.snap | 18 + .../crates_io_database_dump/src/dump-db.toml | 1 + .../down.sql | 36 ++ .../2025-07-22-091706_multiple_emails/up.sql | 102 ++++++ src/controllers/github/secret_scanning.rs | 1 + src/controllers/session.rs | 5 +- src/controllers/user/update.rs | 9 +- ...o__openapi__tests__openapi_snapshot-2.snap | 6 +- src/tests/user.rs | 26 +- src/tests/util/test_app.rs | 1 + src/tests/worker/sync_admins.rs | 1 + src/views.rs | 35 +- src/worker/jobs/expiry_notification.rs | 1 + src/worker/jobs/send_publish_notifications.rs | 1 + 30 files changed, 758 insertions(+), 67 deletions(-) create mode 100644 crates/crates_io_database/tests/email_constraints.rs create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email_when_one_exists.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_same_email_for_different_users.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_with_same_email_as_primary.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_without_primary.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__create_too_many_emails.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__delete_only_email.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__delete_primary_email_with_secondary.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__delete_secondary_email.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__demote_primary_without_new_primary.snap create mode 100644 crates/crates_io_database/tests/snapshots/email_constraints__promote_secondary_to_primary.snap create mode 100644 migrations/2025-07-22-091706_multiple_emails/down.sql create mode 100644 migrations/2025-07-22-091706_multiple_emails/up.sql diff --git a/crates/crates_io_database/src/models/email.rs b/crates/crates_io_database/src/models/email.rs index d3a96cca132..a75d30780f3 100644 --- a/crates/crates_io_database/src/models/email.rs +++ b/crates/crates_io_database/src/models/email.rs @@ -13,6 +13,7 @@ pub struct Email { pub user_id: i32, pub email: String, pub verified: bool, + pub primary: bool, #[diesel(deserialize_as = String, serialize_as = String)] pub token: SecretString, } @@ -24,46 +25,65 @@ pub struct NewEmail<'a> { pub email: &'a str, #[builder(default = false)] pub verified: bool, + #[builder(default = false)] + pub primary: bool, } impl NewEmail<'_> { - pub async fn insert(&self, conn: &mut AsyncPgConnection) -> QueryResult<()> { + pub async fn insert(&self, conn: &mut AsyncPgConnection) -> QueryResult { diesel::insert_into(emails::table) .values(self) - .execute(conn) - .await?; - - Ok(()) + .returning(Email::as_returning()) + .get_result(conn) + .await } - /// Inserts the email into the database and returns the confirmation token, - /// or does nothing if it already exists and returns `None`. - pub async fn insert_if_missing( + /// Inserts the email into the database and returns it, unless the user already has a + /// primary email, in which case it will do nothing and return `None`. + pub async fn insert_primary_if_missing( &self, conn: &mut AsyncPgConnection, - ) -> QueryResult> { - diesel::insert_into(emails::table) - .values(self) - .on_conflict_do_nothing() - .returning(emails::token) - .get_result::(conn) - .await - .map(Into::into) - .optional() + ) -> QueryResult> { + // Check if the user already has a primary email + let primary_count = emails::table + .filter(emails::user_id.eq(self.user_id)) + .filter(emails::primary.eq(true)) + .count() + .get_result::(conn) + .await?; + + if primary_count > 0 { + return Ok(None); // User already has a primary email + } + + self.insert(conn).await.map(Some) } - pub async fn insert_or_update( + // Inserts an email for the user, replacing the primary email if it exists. + pub async fn insert_or_update_primary( &self, conn: &mut AsyncPgConnection, - ) -> QueryResult { - diesel::insert_into(emails::table) - .values(self) - .on_conflict(emails::user_id) - .do_update() - .set(self) - .returning(emails::token) - .get_result::(conn) - .await - .map(Into::into) + ) -> QueryResult { + // Attempt to update an existing primary email + let updated_email = diesel::update( + emails::table + .filter(emails::user_id.eq(self.user_id)) + .filter(emails::primary.eq(true)), + ) + .set(( + emails::email.eq(self.email), + emails::verified.eq(self.verified), + )) + .returning(Email::as_returning()) + .get_result(conn) + .await + .optional()?; + + if let Some(email) = updated_email { + Ok(email) + } else { + // Otherwise, insert a new email + self.insert(conn).await + } } } diff --git a/crates/crates_io_database/src/models/user.rs b/crates/crates_io_database/src/models/user.rs index 946c14301d1..813705d7fdb 100644 --- a/crates/crates_io_database/src/models/user.rs +++ b/crates/crates_io_database/src/models/user.rs @@ -61,8 +61,9 @@ impl User { Ok(users.collect()) } - /// Queries the database for the verified emails - /// belonging to a given user + /// Queries the database for a verified email address belonging to the user. + /// It will ideally return the primary email address if it exists and is + /// verified, otherwise, it will return any verified email address. pub async fn verified_email( &self, conn: &mut AsyncPgConnection, @@ -70,6 +71,7 @@ impl User { Email::belonging_to(self) .select(emails::email) .filter(emails::verified.eq(true)) + .order(emails::primary.desc()) .first(conn) .await .optional() diff --git a/crates/crates_io_database/src/schema.patch b/crates/crates_io_database/src/schema.patch index 18ce21eb9b8..453a14061d9 100644 --- a/crates/crates_io_database/src/schema.patch +++ b/crates/crates_io_database/src/schema.patch @@ -45,6 +45,22 @@ /// The `target` column of the `dependencies` table. /// /// Its SQL type is `Nullable`. +@@ -536,13 +536,14 @@ + /// + /// Its SQL type is `Nullable`. + /// + /// (Automatically generated by Diesel.) + token_generated_at -> Nullable, + /// Whether this email is the primary email address for the user. +- is_primary -> Bool, ++ #[sql_name = "is_primary"] ++ primary -> Bool, + } + } + + diesel::table! { + /// Representation of the `follows` table. + /// @@ -710,6 +702,24 @@ } diff --git a/crates/crates_io_database/src/schema.rs b/crates/crates_io_database/src/schema.rs index e2f2f3cbea0..e34d3c2598b 100644 --- a/crates/crates_io_database/src/schema.rs +++ b/crates/crates_io_database/src/schema.rs @@ -538,6 +538,9 @@ diesel::table! { /// /// (Automatically generated by Diesel.) token_generated_at -> Nullable, + /// Whether this email is the primary email address for the user. + #[sql_name = "is_primary"] + primary -> Bool, } } diff --git a/crates/crates_io_database/tests/email_constraints.rs b/crates/crates_io_database/tests/email_constraints.rs new file mode 100644 index 00000000000..d73c731bcec --- /dev/null +++ b/crates/crates_io_database/tests/email_constraints.rs @@ -0,0 +1,333 @@ +//! Tests to verify that the SQL constraints on the `emails` table are enforced correctly. + +use crates_io_database::models::{Email, NewEmail, NewUser}; +use crates_io_database::schema::{emails, users}; +use crates_io_test_db::TestDatabase; +use diesel::prelude::*; +use diesel::result::Error; +use diesel_async::{AsyncPgConnection, RunQueryDsl}; +use insta::assert_debug_snapshot; + +const MAX_EMAIL_COUNT: i32 = 32; + +#[derive(Debug)] +#[allow(dead_code)] +/// A snapshot of the email data used for testing. +/// This struct is used to compare the results of database operations against expected values. +/// We can't use `Email` directly because it contains date/time fields that would change each time. +struct EmailSnapshot { + id: i32, + user_id: i32, + email: String, + primary: bool, +} +impl From for EmailSnapshot { + fn from(email: Email) -> Self { + EmailSnapshot { + id: email.id, + user_id: email.user_id, + email: email.email, + primary: email.primary, + } + } +} + +// Insert a test user into the database and return its ID. +async fn insert_test_user(conn: &mut AsyncPgConnection) -> i32 { + let user_count = users::table.count().get_result::(conn).await.unwrap(); + let user = NewUser::builder() + .name(&format!("testuser{}", user_count + 1)) + .gh_id(user_count as i32 + 1) + .gh_login(&format!("testuser{}", user_count + 1)) + .gh_access_token("token") + .build() + .insert(conn) + .await + .unwrap(); + user.id +} + +// Insert a basic primary email for a user. +async fn insert_static_primary_email( + conn: &mut AsyncPgConnection, + user_id: i32, +) -> Result { + NewEmail::builder() + .user_id(user_id) + .email("primary@example.com") + .primary(true) + .build() + .insert(conn) + .await +} + +// Insert a basic secondary email for a user. +async fn insert_static_secondary_email( + conn: &mut AsyncPgConnection, + user_id: i32, +) -> Result { + NewEmail::builder() + .user_id(user_id) + .email("secondary@example.com") + .primary(false) + .build() + .insert(conn) + .await +} + +#[tokio::test] +// Add a primary email address to the database. +async fn create_primary_email() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let result = insert_static_primary_email(&mut conn, user_id) + .await + .map(EmailSnapshot::from); + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Attempt to create a secondary email address without a primary already present, which should fail. +// This tests the `verify_exactly_one_primary_email` trigger. +async fn create_secondary_email_without_primary() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let result = insert_static_secondary_email(&mut conn, user_id).await; + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Attempt to delete the only email address for a user, which should succeed. +// This tests the `prevent_primary_email_deletion` trigger. +async fn delete_only_email() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let email = insert_static_primary_email(&mut conn, user_id) + .await + .expect("failed to insert primary email"); + + let result = diesel::delete(emails::table.find(email.id)) + .execute(&mut conn) + .await; + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Add a secondary email address to the database. +async fn create_secondary_email() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let primary = insert_static_primary_email(&mut conn, user_id) + .await + .map(EmailSnapshot::from); + + let secondary = insert_static_secondary_email(&mut conn, user_id) + .await + .map(EmailSnapshot::from); + + assert_debug_snapshot!((primary, secondary)); +} + +#[tokio::test] +// Attempt to delete a secondary email address, which should succeed. +// This tests the `prevent_primary_email_deletion` trigger. +async fn delete_secondary_email() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let _primary = insert_static_primary_email(&mut conn, user_id) + .await + .expect("failed to insert primary email"); + + let secondary = insert_static_secondary_email(&mut conn, user_id) + .await + .expect("failed to insert secondary email"); + + let result = diesel::delete(emails::table.find(secondary.id)) + .execute(&mut conn) + .await; + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Attempt to delete a primary email address when a secondary email exists, which should fail. +// This tests the `prevent_primary_email_deletion` trigger. +async fn delete_primary_email_with_secondary() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let primary = insert_static_primary_email(&mut conn, user_id) + .await + .expect("failed to insert primary email"); + + let _secondary = insert_static_secondary_email(&mut conn, user_id) + .await + .expect("failed to insert secondary email"); + + let result = diesel::delete(emails::table.find(primary.id)) + .execute(&mut conn) + .await; + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Attempt to add a secondary email address with the same email as the primary, which should fail. +// This tests the `unique_user_email` constraint. +async fn create_secondary_email_with_same_email_as_primary() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let primary = insert_static_primary_email(&mut conn, user_id) + .await + .map(EmailSnapshot::from) + .expect("failed to insert primary email"); + + let secondary = NewEmail::builder() + .user_id(user_id) + .email(&primary.email) + .primary(false) + .build() + .insert(&mut conn) + .await + .map(EmailSnapshot::from); + + assert_debug_snapshot!((primary, secondary)); +} + +#[tokio::test] +// Attempt to create more than the maximum allowed emails for a user, which should fail. +// This tests the `enforce_max_emails_per_user` trigger. +async fn create_too_many_emails() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let mut errors = Vec::new(); + for i in 0..MAX_EMAIL_COUNT + 2 { + let result = NewEmail::builder() + .user_id(user_id) + .email(&format!("me+{i}@example.com")) + .primary(i == 0) + .build() + .insert(&mut conn) + .await + .map(EmailSnapshot::from); + + if let Err(err) = result { + errors.push(err); + } + } + + assert_debug_snapshot!(errors); +} + +#[tokio::test] +// Attempt to add the same email address to two users, which should succeed. +async fn create_same_email_for_different_users() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + + let user_id_1: i32 = insert_test_user(&mut conn).await; + let user_id_2: i32 = insert_test_user(&mut conn).await; + + let first = insert_static_primary_email(&mut conn, user_id_1) + .await + .map(EmailSnapshot::from); + + let second = insert_static_primary_email(&mut conn, user_id_2) + .await + .map(EmailSnapshot::from); + + assert_debug_snapshot!((first, second)); +} + +#[tokio::test] +// Create a primary email, a secondary email, and then promote the secondary email to primary. +// This tests the `promote_email_to_primary` function and the `unique_primary_email_per_user` constraint. +async fn promote_secondary_to_primary() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let _primary = insert_static_primary_email(&mut conn, user_id) + .await + .expect("failed to insert primary email"); + + let secondary = insert_static_secondary_email(&mut conn, user_id) + .await + .expect("failed to insert secondary email"); + + diesel::sql_query("SELECT promote_email_to_primary($1)") + .bind::(secondary.id) + .execute(&mut conn) + .await + .expect("failed to promote secondary email to primary"); + + // Query both emails to verify that the primary flag has been updated correctly for both. + let result = emails::table + .select((emails::id, emails::email, emails::primary)) + .load::<(i32, String, bool)>(&mut conn) + .await; + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Attempt to demote a primary email to secondary without assigning another primary, which should fail. +// This tests the `verify_exactly_one_primary_email` trigger. +async fn demote_primary_without_new_primary() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let primary = insert_static_primary_email(&mut conn, user_id) + .await + .expect("failed to insert primary email"); + + let result = diesel::update(emails::table.find(primary.id)) + .set(emails::primary.eq(false)) + .execute(&mut conn) + .await; + + assert_debug_snapshot!(result); +} + +#[tokio::test] +// Attempt to create a primary email when one already exists for the user, which should fail. +// This tests the `unique_primary_email_per_user` constraint. +async fn create_primary_email_when_one_exists() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + let user_id: i32 = insert_test_user(&mut conn).await; + + let first = insert_static_primary_email(&mut conn, user_id) + .await + .map(EmailSnapshot::from); + + let second = NewEmail::builder() + .user_id(user_id) + .email("me+2@example.com") + .primary(true) + .build() + .insert(&mut conn) + .await + .map(EmailSnapshot::from); + + assert_debug_snapshot!((first, second)); +} diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email.snap new file mode 100644 index 00000000000..b8abcdbf777 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email.snap @@ -0,0 +1,12 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Ok( + EmailSnapshot { + id: 1, + user_id: 1, + email: "primary@example.com", + primary: true, + }, +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email_when_one_exists.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email_when_one_exists.snap new file mode 100644 index 00000000000..bd85cf661ae --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_primary_email_when_one_exists.snap @@ -0,0 +1,20 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: "(first, second)" +--- +( + Ok( + EmailSnapshot { + id: 1, + user_id: 1, + email: "primary@example.com", + primary: true, + }, + ), + Err( + DatabaseError( + Unknown, + "User must have one primary email, found 2", + ), + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_same_email_for_different_users.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_same_email_for_different_users.snap new file mode 100644 index 00000000000..be8fdf801f7 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_same_email_for_different_users.snap @@ -0,0 +1,22 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: "(first, second)" +--- +( + Ok( + EmailSnapshot { + id: 1, + user_id: 1, + email: "primary@example.com", + primary: true, + }, + ), + Ok( + EmailSnapshot { + id: 2, + user_id: 2, + email: "primary@example.com", + primary: true, + }, + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email.snap new file mode 100644 index 00000000000..6f2e7065b11 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email.snap @@ -0,0 +1,22 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: "(primary, secondary)" +--- +( + Ok( + EmailSnapshot { + id: 1, + user_id: 1, + email: "primary@example.com", + primary: true, + }, + ), + Ok( + EmailSnapshot { + id: 2, + user_id: 1, + email: "secondary@example.com", + primary: false, + }, + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_with_same_email_as_primary.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_with_same_email_as_primary.snap new file mode 100644 index 00000000000..ff64c3024c3 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_with_same_email_as_primary.snap @@ -0,0 +1,18 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: "(primary, secondary)" +--- +( + EmailSnapshot { + id: 1, + user_id: 1, + email: "primary@example.com", + primary: true, + }, + Err( + DatabaseError( + UniqueViolation, + "duplicate key value violates unique constraint \"unique_user_email\"", + ), + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_without_primary.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_without_primary.snap new file mode 100644 index 00000000000..c0b3d39f0a9 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_secondary_email_without_primary.snap @@ -0,0 +1,10 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Err( + DatabaseError( + Unknown, + "User must have one primary email, found 0", + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__create_too_many_emails.snap b/crates/crates_io_database/tests/snapshots/email_constraints__create_too_many_emails.snap new file mode 100644 index 00000000000..041c49ac10e --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__create_too_many_emails.snap @@ -0,0 +1,10 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: errors +--- +[ + DatabaseError( + Unknown, + "User cannot have more than 32 emails", + ), +] diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__delete_only_email.snap b/crates/crates_io_database/tests/snapshots/email_constraints__delete_only_email.snap new file mode 100644 index 00000000000..bb88b8f7a59 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__delete_only_email.snap @@ -0,0 +1,7 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Ok( + 1, +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__delete_primary_email_with_secondary.snap b/crates/crates_io_database/tests/snapshots/email_constraints__delete_primary_email_with_secondary.snap new file mode 100644 index 00000000000..b7ecff74b7f --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__delete_primary_email_with_secondary.snap @@ -0,0 +1,10 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Err( + DatabaseError( + Unknown, + "Cannot delete primary email. Please set another email as primary first.", + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__delete_secondary_email.snap b/crates/crates_io_database/tests/snapshots/email_constraints__delete_secondary_email.snap new file mode 100644 index 00000000000..bb88b8f7a59 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__delete_secondary_email.snap @@ -0,0 +1,7 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Ok( + 1, +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__demote_primary_without_new_primary.snap b/crates/crates_io_database/tests/snapshots/email_constraints__demote_primary_without_new_primary.snap new file mode 100644 index 00000000000..c0b3d39f0a9 --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__demote_primary_without_new_primary.snap @@ -0,0 +1,10 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Err( + DatabaseError( + Unknown, + "User must have one primary email, found 0", + ), +) diff --git a/crates/crates_io_database/tests/snapshots/email_constraints__promote_secondary_to_primary.snap b/crates/crates_io_database/tests/snapshots/email_constraints__promote_secondary_to_primary.snap new file mode 100644 index 00000000000..b5948db9f8b --- /dev/null +++ b/crates/crates_io_database/tests/snapshots/email_constraints__promote_secondary_to_primary.snap @@ -0,0 +1,18 @@ +--- +source: crates/crates_io_database/tests/email_constraints.rs +expression: result +--- +Ok( + [ + ( + 1, + "primary@example.com", + false, + ), + ( + 2, + "secondary@example.com", + true, + ), + ], +) diff --git a/crates/crates_io_database_dump/src/dump-db.toml b/crates/crates_io_database_dump/src/dump-db.toml index 9394701a49c..184074494b7 100644 --- a/crates/crates_io_database_dump/src/dump-db.toml +++ b/crates/crates_io_database_dump/src/dump-db.toml @@ -141,6 +141,7 @@ id = "private" user_id = "private" email = "private" verified = "private" +is_primary = "private" token = "private" token_generated_at = "private" diff --git a/migrations/2025-07-22-091706_multiple_emails/down.sql b/migrations/2025-07-22-091706_multiple_emails/down.sql new file mode 100644 index 00000000000..4b8ed200540 --- /dev/null +++ b/migrations/2025-07-22-091706_multiple_emails/down.sql @@ -0,0 +1,36 @@ +-- Remove the function for promoting an email to primary +DROP FUNCTION promote_email_to_primary; + +-- Remove the function that enforces the maximum number of emails per user +DROP TRIGGER trigger_enforce_max_emails_per_user ON emails; +DROP FUNCTION enforce_max_emails_per_user(); + +-- Remove the unique constraint for the combination of user_id and email +ALTER TABLE emails DROP CONSTRAINT unique_user_email; + +-- Remove the constraint that allows only one primary email per user +ALTER TABLE emails DROP CONSTRAINT unique_primary_email_per_user; + +-- Remove the trigger that prevents deletion of primary emails +DROP TRIGGER trigger_prevent_primary_email_deletion ON emails; +DROP FUNCTION prevent_primary_email_deletion(); + +-- Remove the trigger that ensures exactly one primary email per user +DROP TRIGGER trigger_verify_exactly_one_primary_email ON emails; +DROP FUNCTION verify_exactly_one_primary_email(); + +-- Remove the primary column from emails table +ALTER TABLE emails DROP COLUMN is_primary; + +-- Remove the GiST extension if it is no longer needed +DROP EXTENSION IF EXISTS btree_gist; + +-- Retain just the first email for each user +DELETE FROM emails +WHERE user_id IN (SELECT user_id FROM emails GROUP BY user_id HAVING COUNT(*) > 1) +AND id NOT IN ( + SELECT MIN(id) FROM emails GROUP BY user_id +); + +-- Re-add the unique constraint on user_id to enforce single email per user +ALTER TABLE emails ADD CONSTRAINT emails_user_id_key UNIQUE (user_id); diff --git a/migrations/2025-07-22-091706_multiple_emails/up.sql b/migrations/2025-07-22-091706_multiple_emails/up.sql new file mode 100644 index 00000000000..db50f27e4a1 --- /dev/null +++ b/migrations/2025-07-22-091706_multiple_emails/up.sql @@ -0,0 +1,102 @@ +-- Drop the unique constraint on user_id to allow multiple emails per user +ALTER TABLE emails DROP CONSTRAINT emails_user_id_key; + +-- Limit users to 32 emails maximum +CREATE FUNCTION enforce_max_emails_per_user() +RETURNS TRIGGER AS $$ +BEGIN + IF (SELECT COUNT(*) FROM emails WHERE user_id = NEW.user_id) > 32 THEN + RAISE EXCEPTION 'User cannot have more than 32 emails'; + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trigger_enforce_max_emails_per_user +BEFORE INSERT ON emails +FOR EACH ROW +EXECUTE FUNCTION enforce_max_emails_per_user(); + +-- Add a unique constraint for the combination of user_id and email +ALTER TABLE emails ADD CONSTRAINT unique_user_email UNIQUE (user_id, email); + +-- Add a new column for identifying the primary email +ALTER TABLE emails ADD COLUMN is_primary BOOLEAN DEFAULT FALSE NOT NULL; +comment on column emails.is_primary is 'Whether this email is the primary email address for the user.'; + +-- Set `is_primary` to true for existing emails +UPDATE emails SET is_primary = true; + +-- Limit primary flag to one email per user +-- Evaluation of the constraint is deferred to the end of the transaction to allow for replacement of the primary email +CREATE EXTENSION IF NOT EXISTS btree_gist; +ALTER TABLE emails ADD CONSTRAINT unique_primary_email_per_user +EXCLUDE USING gist ( + user_id WITH =, + (is_primary::int) WITH = +) +WHERE (is_primary) +DEFERRABLE INITIALLY DEFERRED; + +-- Prevent deletion of primary email, unless it's the only email for that user +CREATE FUNCTION prevent_primary_email_deletion() +RETURNS TRIGGER AS $$ +BEGIN + IF OLD.is_primary IS TRUE THEN + -- Allow deletion if this is the only email for the user + IF (SELECT COUNT(*) FROM emails WHERE user_id = OLD.user_id) = 1 THEN + RETURN OLD; + END IF; + RAISE EXCEPTION 'Cannot delete primary email. Please set another email as primary first.'; + END IF; + RETURN OLD; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trigger_prevent_primary_email_deletion +BEFORE DELETE ON emails +FOR EACH ROW +EXECUTE FUNCTION prevent_primary_email_deletion(); + +-- Ensure exactly one primary email per user after any insert or update +CREATE FUNCTION verify_exactly_one_primary_email() +RETURNS TRIGGER AS $$ +DECLARE + primary_count integer; +BEGIN + -- Count primary emails for the affected user + SELECT COUNT(*) INTO primary_count + FROM emails + WHERE user_id = COALESCE(NEW.user_id, OLD.user_id) + AND is_primary = true; + + IF primary_count != 1 THEN + RAISE EXCEPTION 'User must have one primary email, found %', primary_count; + END IF; + + RETURN COALESCE(NEW, OLD); +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trigger_verify_exactly_one_primary_email +AFTER INSERT OR UPDATE ON emails +FOR EACH ROW +EXECUTE FUNCTION verify_exactly_one_primary_email(); + +-- Function to set the primary flag to true for an existing email +-- This will set the flag to false for all other emails of the same user +CREATE FUNCTION promote_email_to_primary(target_email_id integer) +RETURNS void AS $$ +DECLARE + target_user_id integer; +BEGIN + SELECT user_id INTO target_user_id FROM emails WHERE id = target_email_id; + IF target_user_id IS NULL THEN + RAISE EXCEPTION 'Email ID % does not exist', target_email_id; + END IF; + + UPDATE emails + SET is_primary = (id = target_email_id) + WHERE user_id = target_user_id; +END; +$$ LANGUAGE plpgsql; diff --git a/src/controllers/github/secret_scanning.rs b/src/controllers/github/secret_scanning.rs index 19c5b27a8bf..6829f29d3cc 100644 --- a/src/controllers/github/secret_scanning.rs +++ b/src/controllers/github/secret_scanning.rs @@ -271,6 +271,7 @@ async fn send_trustpub_notification_emails( .filter(crate_owners::deleted.eq(false)) .inner_join(emails::table.on(crate_owners::owner_id.eq(emails::user_id))) .filter(emails::verified.eq(true)) + .filter(emails::primary.eq(true)) .select((crate_owners::crate_id, emails::email)) .order((emails::email, crate_owners::crate_id)) .load::<(i32, String)>(conn) diff --git a/src/controllers/session.rs b/src/controllers/session.rs index 8aaa4753c69..4237791de64 100644 --- a/src/controllers/session.rs +++ b/src/controllers/session.rs @@ -170,15 +170,16 @@ async fn create_or_update_user( let new_email = NewEmail::builder() .user_id(user.id) .email(user_email) + .primary(true) .build(); - if let Some(token) = new_email.insert_if_missing(conn).await? { + if let Some(saved_email) = new_email.insert_primary_if_missing(conn).await? { let email = EmailMessage::from_template( "user_confirm", context! { user_name => user.gh_login, domain => emails.domain, - token => token.expose_secret() + token => saved_email.token.expose_secret() }, ); diff --git a/src/controllers/user/update.rs b/src/controllers/user/update.rs index 7d5174974b1..3021f1829f6 100644 --- a/src/controllers/user/update.rs +++ b/src/controllers/user/update.rs @@ -109,10 +109,13 @@ pub async fn update_user( let new_email = NewEmail::builder() .user_id(user.id) .email(user_email) + .primary(true) .build(); - let token = new_email.insert_or_update(&mut conn).await; - let token = token.map_err(|_| server_error("Error in creating token"))?; + let saved_email = new_email + .insert_or_update_primary(&mut conn) + .await + .map_err(|_| server_error("Error in saving email"))?; // This swallows any errors that occur while attempting to send the email. Some users have // an invalid email set in their GitHub profile, and we should let them sign in even though @@ -123,7 +126,7 @@ pub async fn update_user( context! { user_name => user.gh_login, domain => state.emails.domain, - token => token.expose_secret() + token => saved_email.token.expose_secret() }, ); diff --git a/src/snapshots/crates_io__openapi__tests__openapi_snapshot-2.snap b/src/snapshots/crates_io__openapi__tests__openapi_snapshot-2.snap index 88413740453..2d7e9e9e150 100644 --- a/src/snapshots/crates_io__openapi__tests__openapi_snapshot-2.snap +++ b/src/snapshots/crates_io__openapi__tests__openapi_snapshot-2.snap @@ -88,7 +88,7 @@ expression: response.json() ] }, "email": { - "description": "The user's email address, if set.", + "description": "The user's primary email address, if set.", "example": "kate@morgan.dev", "type": [ "string", @@ -96,12 +96,12 @@ expression: response.json() ] }, "email_verification_sent": { - "description": "Whether the user's email address verification email has been sent.", + "description": "Whether the user's has been sent a verification email to their primary email address, if set.", "example": true, "type": "boolean" }, "email_verified": { - "description": "Whether the user's email address has been verified.", + "description": "Whether the user's primary email address, if set, has been verified.", "example": true, "type": "boolean" }, diff --git a/src/tests/user.rs b/src/tests/user.rs index ac44af2038d..6f758de81e5 100644 --- a/src/tests/user.rs +++ b/src/tests/user.rs @@ -86,7 +86,7 @@ async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> { let json = user_without_github_email.show_me().await; // Check that the setup is correct and the user indeed has no email - assert_eq!(json.user.email, None); + assert_eq!(json.user.primary_email, None); // Add an email address in crates.io user_without_github_email @@ -109,7 +109,7 @@ async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> { let again_user_without_github_email = MockCookieUser::new(&app, u); let json = again_user_without_github_email.show_me().await; - assert_eq!(json.user.email.unwrap(), "apricot@apricots.apricot"); + assert_eq!(json.user.primary_email.unwrap(), "apricot@apricots.apricot"); Ok(()) } @@ -151,7 +151,7 @@ async fn github_with_email_does_not_overwrite_email() -> anyhow::Result<()> { let user_with_different_email_in_github = MockCookieUser::new(&app, u); let json = user_with_different_email_in_github.show_me().await; - assert_eq!(json.user.email, Some(original_email)); + assert_eq!(json.user.primary_email, Some(original_email)); Ok(()) } @@ -164,14 +164,14 @@ async fn test_email_get_and_put() -> anyhow::Result<()> { let (_app, _anon, user) = TestApp::init().with_user().await; let json = user.show_me().await; - assert_eq!(json.user.email.unwrap(), "foo@example.com"); + assert_eq!(json.user.primary_email.unwrap(), "foo@example.com"); user.update_email("mango@mangos.mango").await; let json = user.show_me().await; - assert_eq!(json.user.email.unwrap(), "mango@mangos.mango"); - assert!(!json.user.email_verified); - assert!(json.user.email_verification_sent); + assert_eq!(json.user.primary_email.unwrap(), "mango@mangos.mango"); + assert!(!json.user.primary_email_verified); + assert!(json.user.primary_email_verification_sent); Ok(()) } @@ -216,9 +216,9 @@ async fn test_confirm_user_email() -> anyhow::Result<()> { user.confirm_email(&email_token).await; let json = user.show_me().await; - assert_eq!(json.user.email.unwrap(), "potato2@example.com"); - assert!(json.user.email_verified); - assert!(json.user.email_verification_sent); + assert_eq!(json.user.primary_email.unwrap(), "potato2@example.com"); + assert!(json.user.primary_email_verified); + assert!(json.user.primary_email_verification_sent); Ok(()) } @@ -260,9 +260,9 @@ async fn test_existing_user_email() -> anyhow::Result<()> { let user = MockCookieUser::new(&app, u); let json = user.show_me().await; - assert_eq!(json.user.email.unwrap(), "potahto@example.com"); - assert!(!json.user.email_verified); - assert!(!json.user.email_verification_sent); + assert_eq!(json.user.primary_email.unwrap(), "potahto@example.com"); + assert!(!json.user.primary_email_verified); + assert!(!json.user.primary_email_verification_sent); Ok(()) } diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index 4f2370f7c95..61e6df88ed6 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -140,6 +140,7 @@ impl TestApp { .user_id(user.id) .email(&email) .verified(true) + .primary(true) .build(); new_email.insert(&mut conn).await.unwrap(); diff --git a/src/tests/worker/sync_admins.rs b/src/tests/worker/sync_admins.rs index 3ec4245045d..de89cda254d 100644 --- a/src/tests/worker/sync_admins.rs +++ b/src/tests/worker/sync_admins.rs @@ -90,6 +90,7 @@ async fn create_user( emails::user_id.eq(user_id), emails::email.eq(format!("{name}@crates.io")), emails::verified.eq(true), + emails::primary.eq(true), )) .execute(conn) .await?; diff --git a/src/views.rs b/src/views.rs index 481e3ec50f5..38b0c056bb6 100644 --- a/src/views.rs +++ b/src/views.rs @@ -676,21 +676,24 @@ pub struct EncodablePrivateUser { #[schema(example = "ghost")] pub login: String, - /// Whether the user's email address has been verified. - #[schema(example = true)] - pub email_verified: bool, - - /// Whether the user's email address verification email has been sent. - #[schema(example = true)] - pub email_verification_sent: bool, - /// The user's display name, if set. #[schema(example = "Kate Morgan")] pub name: Option, - /// The user's email address, if set. + /// Whether the user's primary email address, if set, has been verified. + #[schema(example = true)] + #[serde(rename = "email_verified")] + pub primary_email_verified: bool, + + /// Whether the user's has been sent a verification email to their primary email address, if set. + #[schema(example = true)] + #[serde(rename = "email_verification_sent")] + pub primary_email_verification_sent: bool, + + /// The user's primary email address, if set. #[schema(example = "kate@morgan.dev")] - pub email: Option, + #[serde(rename = "email")] + pub primary_email: Option, /// The user's avatar URL, if set. #[schema(example = "https://avatars2.githubusercontent.com/u/1234567?v=4")] @@ -713,9 +716,9 @@ impl EncodablePrivateUser { /// Converts this `User` model into an `EncodablePrivateUser` for JSON serialization. pub fn from( user: User, - email: Option, - email_verified: bool, - email_verification_sent: bool, + primary_email: Option, + primary_email_verified: bool, + primary_email_verification_sent: bool, ) -> Self { let User { id, @@ -730,9 +733,9 @@ impl EncodablePrivateUser { EncodablePrivateUser { id, - email, - email_verified, - email_verification_sent, + primary_email, + primary_email_verified, + primary_email_verification_sent, avatar: gh_avatar, login: gh_login, name, diff --git a/src/worker/jobs/expiry_notification.rs b/src/worker/jobs/expiry_notification.rs index fbcc5e284fc..0f5958f549d 100644 --- a/src/worker/jobs/expiry_notification.rs +++ b/src/worker/jobs/expiry_notification.rs @@ -166,6 +166,7 @@ mod tests { NewEmail::builder() .user_id(user.id) .email("testuser@test.com") + .primary(true) .build() .insert(&mut conn) .await?; diff --git a/src/worker/jobs/send_publish_notifications.rs b/src/worker/jobs/send_publish_notifications.rs index b945456e7dd..7b245720de7 100644 --- a/src/worker/jobs/send_publish_notifications.rs +++ b/src/worker/jobs/send_publish_notifications.rs @@ -59,6 +59,7 @@ impl BackgroundJob for SendPublishNotificationsJob { .filter(users::publish_notifications.eq(true)) .inner_join(emails::table.on(users::id.eq(emails::user_id))) .filter(emails::verified.eq(true)) + .filter(emails::primary.eq(true)) .select((users::gh_login, emails::email)) .load::<(String, String)>(&mut conn) .await?; From 76af4dcc62395648e945fd8ee0f2788a57591ddc Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Fri, 10 Oct 2025 13:41:32 +0100 Subject: [PATCH 2/7] Split email migrations into two phases --- migrations/2025-10-10-123901_primary_email/down.sql | 2 ++ migrations/2025-10-10-123901_primary_email/up.sql | 6 ++++++ .../down.sql | 3 --- .../up.sql | 7 ------- 4 files changed, 8 insertions(+), 10 deletions(-) create mode 100644 migrations/2025-10-10-123901_primary_email/down.sql create mode 100644 migrations/2025-10-10-123901_primary_email/up.sql rename migrations/{2025-07-22-091706_multiple_emails => 2025-10-10-123902_multiple_emails}/down.sql (93%) rename migrations/{2025-07-22-091706_multiple_emails => 2025-10-10-123902_multiple_emails}/up.sql (90%) diff --git a/migrations/2025-10-10-123901_primary_email/down.sql b/migrations/2025-10-10-123901_primary_email/down.sql new file mode 100644 index 00000000000..f2e31e84b4a --- /dev/null +++ b/migrations/2025-10-10-123901_primary_email/down.sql @@ -0,0 +1,2 @@ +-- Remove the primary column from emails table +ALTER TABLE emails DROP COLUMN is_primary; diff --git a/migrations/2025-10-10-123901_primary_email/up.sql b/migrations/2025-10-10-123901_primary_email/up.sql new file mode 100644 index 00000000000..5e9969ef937 --- /dev/null +++ b/migrations/2025-10-10-123901_primary_email/up.sql @@ -0,0 +1,6 @@ +-- Add a new column for identifying the primary email +ALTER TABLE emails ADD COLUMN is_primary BOOLEAN DEFAULT FALSE NOT NULL; +comment on column emails.is_primary is 'Whether this email is the primary email address for the user.'; + +-- After this migration has been applied, please run the following SQL to set the primary email for each user: +-- UPDATE emails SET is_primary = TRUE diff --git a/migrations/2025-07-22-091706_multiple_emails/down.sql b/migrations/2025-10-10-123902_multiple_emails/down.sql similarity index 93% rename from migrations/2025-07-22-091706_multiple_emails/down.sql rename to migrations/2025-10-10-123902_multiple_emails/down.sql index 4b8ed200540..2fd1d59f5ce 100644 --- a/migrations/2025-07-22-091706_multiple_emails/down.sql +++ b/migrations/2025-10-10-123902_multiple_emails/down.sql @@ -19,9 +19,6 @@ DROP FUNCTION prevent_primary_email_deletion(); DROP TRIGGER trigger_verify_exactly_one_primary_email ON emails; DROP FUNCTION verify_exactly_one_primary_email(); --- Remove the primary column from emails table -ALTER TABLE emails DROP COLUMN is_primary; - -- Remove the GiST extension if it is no longer needed DROP EXTENSION IF EXISTS btree_gist; diff --git a/migrations/2025-07-22-091706_multiple_emails/up.sql b/migrations/2025-10-10-123902_multiple_emails/up.sql similarity index 90% rename from migrations/2025-07-22-091706_multiple_emails/up.sql rename to migrations/2025-10-10-123902_multiple_emails/up.sql index db50f27e4a1..77870d8a154 100644 --- a/migrations/2025-07-22-091706_multiple_emails/up.sql +++ b/migrations/2025-10-10-123902_multiple_emails/up.sql @@ -20,13 +20,6 @@ EXECUTE FUNCTION enforce_max_emails_per_user(); -- Add a unique constraint for the combination of user_id and email ALTER TABLE emails ADD CONSTRAINT unique_user_email UNIQUE (user_id, email); --- Add a new column for identifying the primary email -ALTER TABLE emails ADD COLUMN is_primary BOOLEAN DEFAULT FALSE NOT NULL; -comment on column emails.is_primary is 'Whether this email is the primary email address for the user.'; - --- Set `is_primary` to true for existing emails -UPDATE emails SET is_primary = true; - -- Limit primary flag to one email per user -- Evaluation of the constraint is deferred to the end of the transaction to allow for replacement of the primary email CREATE EXTENSION IF NOT EXISTS btree_gist; From de312b7df36cf8383aac8fd42c3fa029933ebcf8 Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Fri, 10 Oct 2025 14:25:22 +0100 Subject: [PATCH 3/7] Remove redundant emails table constraint --- migrations/2025-10-10-123902_multiple_emails/down.sql | 3 --- migrations/2025-10-10-123902_multiple_emails/up.sql | 11 ----------- 2 files changed, 14 deletions(-) diff --git a/migrations/2025-10-10-123902_multiple_emails/down.sql b/migrations/2025-10-10-123902_multiple_emails/down.sql index 2fd1d59f5ce..9c61be162ce 100644 --- a/migrations/2025-10-10-123902_multiple_emails/down.sql +++ b/migrations/2025-10-10-123902_multiple_emails/down.sql @@ -8,9 +8,6 @@ DROP FUNCTION enforce_max_emails_per_user(); -- Remove the unique constraint for the combination of user_id and email ALTER TABLE emails DROP CONSTRAINT unique_user_email; --- Remove the constraint that allows only one primary email per user -ALTER TABLE emails DROP CONSTRAINT unique_primary_email_per_user; - -- Remove the trigger that prevents deletion of primary emails DROP TRIGGER trigger_prevent_primary_email_deletion ON emails; DROP FUNCTION prevent_primary_email_deletion(); diff --git a/migrations/2025-10-10-123902_multiple_emails/up.sql b/migrations/2025-10-10-123902_multiple_emails/up.sql index 77870d8a154..aa30291c8a5 100644 --- a/migrations/2025-10-10-123902_multiple_emails/up.sql +++ b/migrations/2025-10-10-123902_multiple_emails/up.sql @@ -20,17 +20,6 @@ EXECUTE FUNCTION enforce_max_emails_per_user(); -- Add a unique constraint for the combination of user_id and email ALTER TABLE emails ADD CONSTRAINT unique_user_email UNIQUE (user_id, email); --- Limit primary flag to one email per user --- Evaluation of the constraint is deferred to the end of the transaction to allow for replacement of the primary email -CREATE EXTENSION IF NOT EXISTS btree_gist; -ALTER TABLE emails ADD CONSTRAINT unique_primary_email_per_user -EXCLUDE USING gist ( - user_id WITH =, - (is_primary::int) WITH = -) -WHERE (is_primary) -DEFERRABLE INITIALLY DEFERRED; - -- Prevent deletion of primary email, unless it's the only email for that user CREATE FUNCTION prevent_primary_email_deletion() RETURNS TRIGGER AS $$ From f825b4b1ff78286e59649cfcf6e7c1dbfc284286 Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Fri, 10 Oct 2025 14:56:30 +0100 Subject: [PATCH 4/7] Rename `primary` column to `is_primary` --- crates/crates_io_database/src/models/email.rs | 8 +++---- crates/crates_io_database/src/models/user.rs | 2 +- crates/crates_io_database/src/schema.patch | 22 +++---------------- crates/crates_io_database/src/schema.rs | 3 +-- .../tests/email_constraints.rs | 14 ++++++------ src/controllers/github/secret_scanning.rs | 2 +- src/controllers/session.rs | 2 +- src/controllers/user/update.rs | 2 +- src/tests/util/test_app.rs | 2 +- src/tests/worker/sync_admins.rs | 2 +- src/worker/jobs/expiry_notification.rs | 2 +- src/worker/jobs/send_publish_notifications.rs | 2 +- 12 files changed, 23 insertions(+), 40 deletions(-) diff --git a/crates/crates_io_database/src/models/email.rs b/crates/crates_io_database/src/models/email.rs index a75d30780f3..d7bc216f540 100644 --- a/crates/crates_io_database/src/models/email.rs +++ b/crates/crates_io_database/src/models/email.rs @@ -13,7 +13,7 @@ pub struct Email { pub user_id: i32, pub email: String, pub verified: bool, - pub primary: bool, + pub is_primary: bool, #[diesel(deserialize_as = String, serialize_as = String)] pub token: SecretString, } @@ -26,7 +26,7 @@ pub struct NewEmail<'a> { #[builder(default = false)] pub verified: bool, #[builder(default = false)] - pub primary: bool, + pub is_primary: bool, } impl NewEmail<'_> { @@ -47,7 +47,7 @@ impl NewEmail<'_> { // Check if the user already has a primary email let primary_count = emails::table .filter(emails::user_id.eq(self.user_id)) - .filter(emails::primary.eq(true)) + .filter(emails::is_primary.eq(true)) .count() .get_result::(conn) .await?; @@ -68,7 +68,7 @@ impl NewEmail<'_> { let updated_email = diesel::update( emails::table .filter(emails::user_id.eq(self.user_id)) - .filter(emails::primary.eq(true)), + .filter(emails::is_primary.eq(true)), ) .set(( emails::email.eq(self.email), diff --git a/crates/crates_io_database/src/models/user.rs b/crates/crates_io_database/src/models/user.rs index 813705d7fdb..aaf0164090c 100644 --- a/crates/crates_io_database/src/models/user.rs +++ b/crates/crates_io_database/src/models/user.rs @@ -71,7 +71,7 @@ impl User { Email::belonging_to(self) .select(emails::email) .filter(emails::verified.eq(true)) - .order(emails::primary.desc()) + .order(emails::is_primary.desc()) .first(conn) .await .optional() diff --git a/crates/crates_io_database/src/schema.patch b/crates/crates_io_database/src/schema.patch index 453a14061d9..1f1db2b668a 100644 --- a/crates/crates_io_database/src/schema.patch +++ b/crates/crates_io_database/src/schema.patch @@ -9,7 +9,7 @@ - pub struct Tsvector; + pub use diesel_full_text_search::Tsvector; } - + diesel::table! { @@ -67,9 +65,9 @@ /// (Automatically generated by Diesel.) @@ -35,7 +35,7 @@ - path -> Ltree, } } - + @@ -483,7 +475,7 @@ /// Its SQL type is `Array>`. /// @@ -45,25 +45,9 @@ /// The `target` column of the `dependencies` table. /// /// Its SQL type is `Nullable`. -@@ -536,13 +536,14 @@ - /// - /// Its SQL type is `Nullable`. - /// - /// (Automatically generated by Diesel.) - token_generated_at -> Nullable, - /// Whether this email is the primary email address for the user. -- is_primary -> Bool, -+ #[sql_name = "is_primary"] -+ primary -> Bool, - } - } - - diesel::table! { - /// Representation of the `follows` table. - /// @@ -710,6 +702,24 @@ } - + diesel::table! { + /// Representation of the `recent_crate_downloads` view. + /// diff --git a/crates/crates_io_database/src/schema.rs b/crates/crates_io_database/src/schema.rs index e34d3c2598b..09065169035 100644 --- a/crates/crates_io_database/src/schema.rs +++ b/crates/crates_io_database/src/schema.rs @@ -539,8 +539,7 @@ diesel::table! { /// (Automatically generated by Diesel.) token_generated_at -> Nullable, /// Whether this email is the primary email address for the user. - #[sql_name = "is_primary"] - primary -> Bool, + is_primary -> Bool, } } diff --git a/crates/crates_io_database/tests/email_constraints.rs b/crates/crates_io_database/tests/email_constraints.rs index d73c731bcec..a898a445231 100644 --- a/crates/crates_io_database/tests/email_constraints.rs +++ b/crates/crates_io_database/tests/email_constraints.rs @@ -55,7 +55,7 @@ async fn insert_static_primary_email( NewEmail::builder() .user_id(user_id) .email("primary@example.com") - .primary(true) + .is_primary(true) .build() .insert(conn) .await @@ -69,7 +69,7 @@ async fn insert_static_secondary_email( NewEmail::builder() .user_id(user_id) .email("secondary@example.com") - .primary(false) + .is_primary(false) .build() .insert(conn) .await @@ -201,7 +201,7 @@ async fn create_secondary_email_with_same_email_as_primary() { let secondary = NewEmail::builder() .user_id(user_id) .email(&primary.email) - .primary(false) + .is_primary(false) .build() .insert(&mut conn) .await @@ -223,7 +223,7 @@ async fn create_too_many_emails() { let result = NewEmail::builder() .user_id(user_id) .email(&format!("me+{i}@example.com")) - .primary(i == 0) + .is_primary(i == 0) .build() .insert(&mut conn) .await @@ -281,7 +281,7 @@ async fn promote_secondary_to_primary() { // Query both emails to verify that the primary flag has been updated correctly for both. let result = emails::table - .select((emails::id, emails::email, emails::primary)) + .select((emails::id, emails::email, emails::is_primary)) .load::<(i32, String, bool)>(&mut conn) .await; @@ -301,7 +301,7 @@ async fn demote_primary_without_new_primary() { .expect("failed to insert primary email"); let result = diesel::update(emails::table.find(primary.id)) - .set(emails::primary.eq(false)) + .set(emails::is_primary.eq(false)) .execute(&mut conn) .await; @@ -323,7 +323,7 @@ async fn create_primary_email_when_one_exists() { let second = NewEmail::builder() .user_id(user_id) .email("me+2@example.com") - .primary(true) + .is_primary(true) .build() .insert(&mut conn) .await diff --git a/src/controllers/github/secret_scanning.rs b/src/controllers/github/secret_scanning.rs index 6829f29d3cc..12102d7c748 100644 --- a/src/controllers/github/secret_scanning.rs +++ b/src/controllers/github/secret_scanning.rs @@ -271,7 +271,7 @@ async fn send_trustpub_notification_emails( .filter(crate_owners::deleted.eq(false)) .inner_join(emails::table.on(crate_owners::owner_id.eq(emails::user_id))) .filter(emails::verified.eq(true)) - .filter(emails::primary.eq(true)) + .filter(emails::is_primary.eq(true)) .select((crate_owners::crate_id, emails::email)) .order((emails::email, crate_owners::crate_id)) .load::<(i32, String)>(conn) diff --git a/src/controllers/session.rs b/src/controllers/session.rs index 4237791de64..c42dea30a79 100644 --- a/src/controllers/session.rs +++ b/src/controllers/session.rs @@ -170,7 +170,7 @@ async fn create_or_update_user( let new_email = NewEmail::builder() .user_id(user.id) .email(user_email) - .primary(true) + .is_primary(true) .build(); if let Some(saved_email) = new_email.insert_primary_if_missing(conn).await? { diff --git a/src/controllers/user/update.rs b/src/controllers/user/update.rs index 3021f1829f6..1a3fedab492 100644 --- a/src/controllers/user/update.rs +++ b/src/controllers/user/update.rs @@ -109,7 +109,7 @@ pub async fn update_user( let new_email = NewEmail::builder() .user_id(user.id) .email(user_email) - .primary(true) + .is_primary(true) .build(); let saved_email = new_email diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index 61e6df88ed6..daf5384e1d1 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -140,7 +140,7 @@ impl TestApp { .user_id(user.id) .email(&email) .verified(true) - .primary(true) + .is_primary(true) .build(); new_email.insert(&mut conn).await.unwrap(); diff --git a/src/tests/worker/sync_admins.rs b/src/tests/worker/sync_admins.rs index de89cda254d..02445d6f628 100644 --- a/src/tests/worker/sync_admins.rs +++ b/src/tests/worker/sync_admins.rs @@ -90,7 +90,7 @@ async fn create_user( emails::user_id.eq(user_id), emails::email.eq(format!("{name}@crates.io")), emails::verified.eq(true), - emails::primary.eq(true), + emails::is_primary.eq(true), )) .execute(conn) .await?; diff --git a/src/worker/jobs/expiry_notification.rs b/src/worker/jobs/expiry_notification.rs index 0f5958f549d..4af684f6313 100644 --- a/src/worker/jobs/expiry_notification.rs +++ b/src/worker/jobs/expiry_notification.rs @@ -166,7 +166,7 @@ mod tests { NewEmail::builder() .user_id(user.id) .email("testuser@test.com") - .primary(true) + .is_primary(true) .build() .insert(&mut conn) .await?; diff --git a/src/worker/jobs/send_publish_notifications.rs b/src/worker/jobs/send_publish_notifications.rs index 7b245720de7..54f974f1d8b 100644 --- a/src/worker/jobs/send_publish_notifications.rs +++ b/src/worker/jobs/send_publish_notifications.rs @@ -59,7 +59,7 @@ impl BackgroundJob for SendPublishNotificationsJob { .filter(users::publish_notifications.eq(true)) .inner_join(emails::table.on(users::id.eq(emails::user_id))) .filter(emails::verified.eq(true)) - .filter(emails::primary.eq(true)) + .filter(emails::is_primary.eq(true)) .select((users::gh_login, emails::email)) .load::<(String, String)>(&mut conn) .await?; From 42572721d2bf5039b541a2cdb9b0a1cb63afed6c Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Fri, 10 Oct 2025 15:00:44 +0100 Subject: [PATCH 5/7] Update `primary` row name in email constraint tests --- crates/crates_io_database/tests/email_constraints.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/crates_io_database/tests/email_constraints.rs b/crates/crates_io_database/tests/email_constraints.rs index a898a445231..aa5e7121ad1 100644 --- a/crates/crates_io_database/tests/email_constraints.rs +++ b/crates/crates_io_database/tests/email_constraints.rs @@ -27,7 +27,7 @@ impl From for EmailSnapshot { id: email.id, user_id: email.user_id, email: email.email, - primary: email.primary, + primary: email.is_primary, } } } @@ -310,7 +310,7 @@ async fn demote_primary_without_new_primary() { #[tokio::test] // Attempt to create a primary email when one already exists for the user, which should fail. -// This tests the `unique_primary_email_per_user` constraint. +// This tests the `verify_exactly_one_primary_email` constraint. async fn create_primary_email_when_one_exists() { let test_db = TestDatabase::new(); let mut conn = test_db.async_connect().await; From d9fb4f3e2085ff587d6673e70883b5b0aca851c2 Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Fri, 10 Oct 2025 15:03:53 +0100 Subject: [PATCH 6/7] Raise an error when attempting to `insert_or_update_primary` on a non-primary email --- crates/crates_io_database/src/models/email.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/crates_io_database/src/models/email.rs b/crates/crates_io_database/src/models/email.rs index d7bc216f540..5b877f2b541 100644 --- a/crates/crates_io_database/src/models/email.rs +++ b/crates/crates_io_database/src/models/email.rs @@ -64,6 +64,12 @@ impl NewEmail<'_> { &self, conn: &mut AsyncPgConnection, ) -> QueryResult { + if self.is_primary { + return Err(diesel::result::Error::QueryBuilderError( + "Cannot use insert_or_update_primary with a non-primary email".into(), + )); + } + // Attempt to update an existing primary email let updated_email = diesel::update( emails::table From daac601e34b59f9b5cadf8cfb5e8d8b41c648924 Mon Sep 17 00:00:00 2001 From: Kailan Blanks Date: Fri, 10 Oct 2025 15:16:57 +0100 Subject: [PATCH 7/7] Replace usage of deprecated `gh_access_token` field in model test --- crates/crates_io_database/tests/email_constraints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/crates_io_database/tests/email_constraints.rs b/crates/crates_io_database/tests/email_constraints.rs index aa5e7121ad1..bebb549095c 100644 --- a/crates/crates_io_database/tests/email_constraints.rs +++ b/crates/crates_io_database/tests/email_constraints.rs @@ -39,7 +39,7 @@ async fn insert_test_user(conn: &mut AsyncPgConnection) -> i32 { .name(&format!("testuser{}", user_count + 1)) .gh_id(user_count as i32 + 1) .gh_login(&format!("testuser{}", user_count + 1)) - .gh_access_token("token") + .gh_encrypted_token(&[]) .build() .insert(conn) .await