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?;