diff --git a/.gitignore b/.gitignore index 1947c24fc9d..b18dca911f9 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,7 @@ src/schema.rs.orig # insta *.pending-snap +*.snap.new # playwright /test-results/ diff --git a/app/components/email-input.hbs b/app/components/email-input.hbs index d212ffdf4b0..e28db2ef722 100644 --- a/app/components/email-input.hbs +++ b/app/components/email-input.hbs @@ -1,101 +1,64 @@
- {{#unless @user.email}} -
-

- Please add your email address. We will only use - it to contact you about your account. We promise we'll never share it! -

+ {{#if this.email.id }} +
+
+
+ {{ this.email.email }} + + {{#if this.email.verified}} + Verified + {{#if this.email.primary }} + Primary email + {{/if}} + {{else}} + {{#if this.email.verification_email_sent}} + Unverified - email sent + {{else}} + Unverified + {{/if}} + {{/if}} + +
- {{/unless}} - - {{#if this.isEditing }} -
-
- -
-
- - -
- - - -
-
+
+ {{#unless this.email.verified}} + + {{/unless}} + {{#if (and (not this.email.primary) this.email.verified)}} + + {{/if}} + {{#if @canDelete}} + + {{/if}}
+
{{else}} -
-
-
Email
-
-
-
- {{ @user.email }} - {{#if @user.email_verified}} - Verified! - {{/if}} -
-
+
+
+ +
-
-
- {{#if (and @user.email (not @user.email_verified))}} -
-
- {{#if @user.email_verification_sent}} -

We have sent a verification email to your address.

- {{/if}} -

Your email has not yet been verified.

-
-
- -
-
- {{/if}} + +
{{/if}} -
\ No newline at end of file +
diff --git a/app/components/email-input.js b/app/components/email-input.js index 993958d9bef..5e4e2c90b46 100644 --- a/app/components/email-input.js +++ b/app/components/email-input.js @@ -8,13 +8,18 @@ import { task } from 'ember-concurrency'; export default class EmailInput extends Component { @service notifications; + @tracked email = this.args.email || { email: '', id: null }; + @tracked isValid = false; @tracked value; - @tracked isEditing = false; @tracked disableResend = false; + @action validate(event) { + this.isValid = event.target.value.trim().length !== 0 && event.target.checkValidity(); + } + resendEmailTask = task(async () => { try { - await this.args.user.resendVerificationEmail(); + await this.args.user.resendVerificationEmail(this.email.id); this.disableResend = true; } catch (error) { let detail = error.errors?.[0]?.detail; @@ -26,30 +31,47 @@ export default class EmailInput extends Component { } }); - @action - editEmail() { - this.value = this.args.user.email; - this.isEditing = true; - } + deleteEmailTask = task(async () => { + try { + await this.args.user.deleteEmail(this.email.id); + } catch (error) { + console.error('Error deleting email:', error); + let detail = error.errors?.[0]?.detail; + if (detail && !detail.startsWith('{')) { + this.notifications.error(`Error in deleting email: ${detail}`); + } else { + this.notifications.error('Unknown error in deleting email'); + } + } + }); saveEmailTask = task(async () => { - let userEmail = this.value; - let user = this.args.user; - try { - await user.changeEmail(userEmail); - - this.isEditing = false; - this.disableResend = false; + this.email = await this.args.user.addEmail(this.value); + this.disableResend = true; + await this.args.onAddEmail?.(); } catch (error) { let detail = error.errors?.[0]?.detail; - let msg = - detail && !detail.startsWith('{') - ? `An error occurred while saving this email, ${detail}` - : 'An unknown error occurred while saving this email.'; + if (detail && !detail.startsWith('{')) { + this.notifications.error(`Error in saving email: ${detail}`); + } else { + console.error('Error saving email:', error); + this.notifications.error('Unknown error in saving email'); + } + } + }); - this.notifications.error(`Error in saving email: ${msg}`); + markAsPrimaryTask = task(async () => { + try { + await this.args.user.updatePrimaryEmail(this.email.id); + } catch (error) { + let detail = error.errors?.[0]?.detail; + if (detail && !detail.startsWith('{')) { + this.notifications.error(`Error in marking email as primary: ${detail}`); + } else { + this.notifications.error('Unknown error in marking email as primary'); + } } }); } diff --git a/app/components/email-input.module.css b/app/components/email-input.module.css index 1313e662caa..14522ac9c10 100644 --- a/app/components/email-input.module.css +++ b/app/components/email-input.module.css @@ -1,7 +1,3 @@ -.friendly-message { - margin-top: 0; -} - .row { width: 100%; border: 1px solid var(--gray-border); @@ -9,6 +5,7 @@ padding: var(--space-2xs) var(--space-s); display: flex; align-items: center; + justify-content: space-between; &:last-child { border-bottom-width: 1px; @@ -22,12 +19,41 @@ } .email-column { + padding: var(--space-xs) 0; +} + +.email-column dd { + margin: 0; + display: flex; + flex-wrap: wrap; + gap: var(--space-3xs); flex: 20; } +.email-column .badges { + display: flex; + flex-wrap: wrap; + gap: var(--space-3xs); +} + +.badge { + padding: var(--space-4xs) var(--space-2xs); + background-color: var(--main-bg-dark); + font-size: 0.8rem; + border-radius: 100px; +} + .verified { - color: green; - font-weight: bold; + background-color: var(--green800); + color: var(--grey200); +} + +.pending-verification { + background-color: light-dark(var(--orange-200), var(--orange-500)); +} + +.unverified { + background-color: light-dark(var(--orange-300), var(--orange-600)); } .email-form { @@ -38,13 +64,22 @@ } .input { - width: 400px; + background-color: var(--main-bg); + border: 0; + flex: 1; + margin: calc(var(--space-3xs) * -1) calc(var(--space-2xs) * -1); + padding: var(--space-3xs) var(--space-2xs); margin-right: var(--space-xs); } +.input:focus { + outline: none; +} + .actions { display: flex; align-items: center; + gap: var(--space-3xs); } .save-button { diff --git a/app/controllers/settings/profile.js b/app/controllers/settings/profile.js index ad0649ce9a4..b16f86e8372 100644 --- a/app/controllers/settings/profile.js +++ b/app/controllers/settings/profile.js @@ -8,6 +8,8 @@ import { task } from 'ember-concurrency'; export default class extends Controller { @service notifications; + @tracked isAddingEmail = false; + @tracked publishNotifications; @action handleNotificationsChange(event) { diff --git a/app/models/user.js b/app/models/user.js index 1c7b51d72d7..62dc2f62481 100644 --- a/app/models/user.js +++ b/app/models/user.js @@ -7,9 +7,7 @@ import { apiAction } from '@mainmatter/ember-api-actions'; export default class User extends Model { @service store; - @attr email; - @attr email_verified; - @attr email_verification_sent; + @attr emails; @attr name; @attr is_admin; @attr login; @@ -22,15 +20,45 @@ export default class User extends Model { return await waitForPromise(apiAction(this, { method: 'GET', path: 'stats' })); } - async changeEmail(email) { - await waitForPromise(apiAction(this, { method: 'PUT', data: { user: { email } } })); + async addEmail(emailAddress) { + let email = await waitForPromise( + apiAction(this, { + method: 'POST', + path: 'emails', + data: { email: emailAddress }, + }), + ); this.store.pushPayload({ user: { id: this.id, - email, - email_verified: false, - email_verification_sent: true, + emails: [...this.emails, email], + }, + }); + } + + async resendVerificationEmail(emailId) { + return await waitForPromise(apiAction(this, { method: 'PUT', path: `emails/${emailId}/resend` })); + } + + async deleteEmail(emailId) { + await waitForPromise(apiAction(this, { method: 'DELETE', path: `emails/${emailId}` })); + + this.store.pushPayload({ + user: { + id: this.id, + emails: this.emails.filter(email => email.id !== emailId), + }, + }); + } + + async updatePrimaryEmail(emailId) { + await waitForPromise(apiAction(this, { method: 'PUT', path: `emails/${emailId}/set_primary` })); + + this.store.pushPayload({ + user: { + id: this.id, + emails: this.emails.map(email => ({ ...email, primary: email.id === emailId })), }, }); } @@ -45,8 +73,4 @@ export default class User extends Model { }, }); } - - async resendVerificationEmail() { - return await waitForPromise(apiAction(this, { method: 'PUT', path: 'resend' })); - } } diff --git a/app/routes/confirm.js b/app/routes/confirm.js index 1ef7fc2142e..16ecf8dbac8 100644 --- a/app/routes/confirm.js +++ b/app/routes/confirm.js @@ -11,14 +11,22 @@ export default class ConfirmRoute extends Route { async model(params) { try { - await ajax(`/api/v1/confirm/${params.email_token}`, { method: 'PUT', body: '{}' }); + let response = await ajax(`/api/v1/confirm/${params.email_token}`, { method: 'PUT', body: '{}' }); // wait for the `GET /api/v1/me` call to complete before // trying to update the Ember Data store await this.session.loadUserTask.last; if (this.session.currentUser) { - this.store.pushPayload({ user: { id: this.session.currentUser.id, email_verified: true } }); + this.store.pushPayload({ + user: { + id: this.session.currentUser.id, + emails: [ + ...this.session.currentUser.emails.filter(email => email.id !== response.email.id), + response.email, + ].sort((a, b) => a.id - b.id), + }, + }); } this.notifications.success('Thank you for confirming your email! :)'); diff --git a/app/routes/settings/profile.js b/app/routes/settings/profile.js index bb4faabbd53..84f54e18a16 100644 --- a/app/routes/settings/profile.js +++ b/app/routes/settings/profile.js @@ -12,5 +12,6 @@ export default class ProfileSettingsRoute extends AuthenticatedRoute { setupController(controller, model) { super.setupController(...arguments); controller.publishNotifications = model.user.publish_notifications; + controller.primaryEmailId = model.user.emails.find(email => email.primary)?.id; } } diff --git a/app/styles/settings/profile.module.css b/app/styles/settings/profile.module.css index bdabd5a791c..ee59e7071f4 100644 --- a/app/styles/settings/profile.module.css +++ b/app/styles/settings/profile.module.css @@ -53,6 +53,26 @@ column-gap: var(--space-xs); } +.friendly-message { + margin: 0; + margin-bottom: var(--space-s); +} + +.email-selector { + display: flex; + flex-direction: column; + margin-bottom: var(--space-s); +} + +.select-label { + margin-bottom: var(--space-3xs); +} + +.add-email { + margin-top: var(--space-xs); + display: flex; +} + .label { grid-area: label; font-weight: bold; diff --git a/app/templates/crate/settings/index.hbs b/app/templates/crate/settings/index.hbs index f5321ee8be2..1f37235150b 100644 --- a/app/templates/crate/settings/index.hbs +++ b/app/templates/crate/settings/index.hbs @@ -47,7 +47,11 @@ {{/if}}
- {{user.email}} + {{#each user.emails as |email|}} + {{#if email.primary}} + {{email.email}} + {{/if}} + {{/each}}
diff --git a/app/templates/settings/email-notifications.hbs b/app/templates/settings/email-notifications.hbs index e0c78191e29..41045a69e16 100644 --- a/app/templates/settings/email-notifications.hbs +++ b/app/templates/settings/email-notifications.hbs @@ -49,4 +49,4 @@

{{/if}} - \ No newline at end of file + diff --git a/app/templates/settings/profile.hbs b/app/templates/settings/profile.hbs index cc33b80ec67..23ca904e415 100644 --- a/app/templates/settings/profile.hbs +++ b/app/templates/settings/profile.hbs @@ -25,13 +25,38 @@
-

User Email

- +

User Emails

+ {{#if (eq this.model.user.emails.length 0)}} +

+ Please add your email address. We will only use + it to contact you about your account. We promise we'll never share it! +

+ {{/if}} + {{#each this.model.user.emails as |email|}} + + {{/each}} + {{#if this.isAddingEmail}} + + {{else}} +
+ +
+ {{/if}}
+ {{#unless (lt this.model.user.emails.length 1)}}

Notification Settings

@@ -44,7 +69,7 @@ /> Publish Notifications - Publish notifications are sent to your email address whenever new + Publish notifications are sent to your primary email address whenever new versions of a crate that you own are published. These can be useful to quickly detect compromised accounts or API tokens. @@ -64,4 +89,5 @@ {{/if}}
- \ No newline at end of file + {{/unless}} + diff --git a/crates/crates_io_database/src/models/email.rs b/crates/crates_io_database/src/models/email.rs index d3a96cca132..4e879486d49 100644 --- a/crates/crates_io_database/src/models/email.rs +++ b/crates/crates_io_database/src/models/email.rs @@ -1,5 +1,7 @@ use bon::Builder; +use chrono::{DateTime, Utc}; use diesel::prelude::*; +use diesel::upsert::on_constraint; use diesel_async::{AsyncPgConnection, RunQueryDsl}; use secrecy::SecretString; @@ -13,8 +15,20 @@ 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, + pub token_generated_at: Option>, +} + +impl Email { + pub async fn find(conn: &mut AsyncPgConnection, id: i32) -> QueryResult { + emails::table + .find(id) + .select(Email::as_select()) + .first(conn) + .await + } } #[derive(Debug, Insertable, AsChangeset, Builder)] @@ -24,46 +38,32 @@ 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, + /// Inserts the email into the database and returns the email record, /// or does nothing if it already exists and returns `None`. pub async fn insert_if_missing( &self, conn: &mut AsyncPgConnection, - ) -> QueryResult> { + ) -> QueryResult> { diesel::insert_into(emails::table) .values(self) - .on_conflict_do_nothing() - .returning(emails::token) - .get_result::(conn) + .on_conflict(on_constraint("unique_user_email")) + .do_nothing() + .returning(Email::as_returning()) + .get_result(conn) .await - .map(Into::into) .optional() } - - pub async fn insert_or_update( - &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) - } } 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..2d8e637ae4e 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,9 +45,25 @@ /// 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 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/crates/crates_io_github/src/lib.rs b/crates/crates_io_github/src/lib.rs index e7656bd70b5..d352618e95a 100644 --- a/crates/crates_io_github/src/lib.rs +++ b/crates/crates_io_github/src/lib.rs @@ -20,6 +20,7 @@ type Result = std::result::Result; #[async_trait] pub trait GitHubClient: Send + Sync { async fn current_user(&self, auth: &AccessToken) -> Result; + async fn current_user_emails(&self, auth: &AccessToken) -> Result>; async fn get_user(&self, name: &str, auth: &AccessToken) -> Result; async fn org_by_name(&self, org_name: &str, auth: &AccessToken) -> Result; async fn team_by_name( @@ -103,6 +104,10 @@ impl GitHubClient for RealGitHubClient { self.request("/user", auth).await } + async fn current_user_emails(&self, auth: &AccessToken) -> Result> { + self.request("/user/emails", auth).await + } + async fn get_user(&self, name: &str, auth: &AccessToken) -> Result { let url = format!("/users/{name}"); self.request(&url, auth).await @@ -197,6 +202,13 @@ pub struct GitHubUser { pub name: Option, } +#[derive(Debug, Deserialize)] +pub struct GitHubEmail { + pub email: String, + pub primary: bool, + pub verified: bool, +} + #[derive(Debug, Deserialize)] pub struct GitHubOrganization { pub id: i32, // unique GH id (needed for membership queries) diff --git a/e2e/acceptance/email-change.spec.ts b/e2e/acceptance/email-change.spec.ts deleted file mode 100644 index b152bf2bf12..00000000000 --- a/e2e/acceptance/email-change.spec.ts +++ /dev/null @@ -1,179 +0,0 @@ -import { expect, test } from '@/e2e/helper'; -import { http, HttpResponse } from 'msw'; - -test.describe('Acceptance | Email Change', { tag: '@acceptance' }, () => { - test('happy path', async ({ page, msw }) => { - let user = msw.db.user.create({ email: 'old@email.com' }); - await msw.authenticateAs(user); - - await page.goto('/settings/profile'); - await expect(page).toHaveURL('/settings/profile'); - const emailInput = page.locator('[data-test-email-input]'); - await expect(emailInput).toBeVisible(); - await expect(emailInput.locator('[data-test-no-email]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-email-address]')).toContainText('old@email.com'); - await expect(emailInput.locator('[data-test-verified]')).toBeVisible(); - await expect(emailInput.locator('[data-test-not-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-verification-sent]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-resend-button]')).toHaveCount(0); - - await emailInput.locator('[data-test-edit-button]').click(); - await expect(emailInput.locator('[data-test-input]')).toHaveValue('old@email.com'); - await expect(emailInput.locator('[data-test-save-button]')).toBeEnabled(); - await expect(emailInput.locator('[data-test-cancel-button]')).toBeEnabled(); - - await emailInput.locator('[data-test-input]').fill(''); - await expect(emailInput.locator('[data-test-input]')).toHaveValue(''); - await expect(emailInput.locator('[data-test-save-button]')).toBeDisabled(); - - await emailInput.locator('[data-test-input]').fill('new@email.com'); - await expect(emailInput.locator('[data-test-input]')).toHaveValue('new@email.com'); - await expect(emailInput.locator('[data-test-save-button]')).toBeEnabled(); - - await emailInput.locator('[data-test-save-button]').click(); - await expect(emailInput.locator('[data-test-email-address]')).toContainText('new@email.com'); - await expect(emailInput.locator('[data-test-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-not-verified]')).toBeVisible(); - await expect(emailInput.locator('[data-test-verification-sent]')).toBeVisible(); - await expect(emailInput.locator('[data-test-resend-button]')).toBeEnabled(); - - user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); - await expect(user.email).toBe('new@email.com'); - await expect(user.emailVerified).toBe(false); - await expect(user.emailVerificationToken).toBeDefined(); - }); - - test('happy path with `email: null`', async ({ page, msw }) => { - let user = msw.db.user.create({ email: undefined }); - await msw.authenticateAs(user); - - await page.goto('/settings/profile'); - await expect(page).toHaveURL('/settings/profile'); - const emailInput = page.locator('[data-test-email-input]'); - await expect(emailInput).toBeVisible(); - await expect(emailInput.locator('[data-test-no-email]')).toBeVisible(); - await expect(emailInput.locator('[data-test-email-address]')).toHaveText(''); - await expect(emailInput.locator('[data-test-not-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-verification-sent]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-resend-button]')).toHaveCount(0); - - await emailInput.locator('[data-test-edit-button]').click(); - await expect(emailInput.locator('[data-test-input]')).toHaveValue(''); - await expect(emailInput.locator('[data-test-save-button]')).toBeDisabled(); - await expect(emailInput.locator('[data-test-cancel-button]')).toBeEnabled(); - - await emailInput.locator('[data-test-input]').fill('new@email.com'); - await expect(emailInput.locator('[data-test-input]')).toHaveValue('new@email.com'); - await expect(emailInput.locator('[data-test-save-button]')).toBeEnabled(); - - await emailInput.locator('[data-test-save-button]').click(); - await expect(emailInput.locator('[data-test-no-email]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-email-address]')).toContainText('new@email.com'); - await expect(emailInput.locator('[data-test-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-not-verified]')).toBeVisible(); - await expect(emailInput.locator('[data-test-verification-sent]')).toBeVisible(); - await expect(emailInput.locator('[data-test-resend-button]')).toBeEnabled(); - - user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); - await expect(user.email).toBe('new@email.com'); - await expect(user.emailVerified).toBe(false); - await expect(user.emailVerificationToken).toBeDefined(); - }); - - test('cancel button', async ({ page, msw }) => { - let user = msw.db.user.create({ email: 'old@email.com' }); - await msw.authenticateAs(user); - - await page.goto('/settings/profile'); - const emailInput = page.locator('[data-test-email-input]'); - await emailInput.locator('[data-test-edit-button]').click(); - await emailInput.locator('[data-test-input]').fill('new@email.com'); - await expect(emailInput.locator('[data-test-invalid-email-warning]')).toHaveCount(0); - - await emailInput.locator('[data-test-cancel-button]').click(); - await expect(emailInput.locator('[data-test-email-address]')).toContainText('old@email.com'); - await expect(emailInput.locator('[data-test-verified]')).toBeVisible(); - await expect(emailInput.locator('[data-test-not-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-verification-sent]')).toHaveCount(0); - - user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); - await expect(user.email).toBe('old@email.com'); - await expect(user.emailVerified).toBe(true); - await expect(user.emailVerificationToken).toBe(null); - }); - - test('server error', async ({ page, msw }) => { - let user = msw.db.user.create({ email: 'old@email.com' }); - await msw.authenticateAs(user); - - let error = HttpResponse.json({}, { status: 500 }); - await msw.worker.use(http.put('/api/v1/users/:user_id', () => error)); - - await page.goto('/settings/profile'); - const emailInput = page.locator('[data-test-email-input]'); - await emailInput.locator('[data-test-edit-button]').click(); - await emailInput.locator('[data-test-input]').fill('new@email.com'); - - await emailInput.locator('[data-test-save-button]').click(); - await expect(emailInput.locator('[data-test-input]')).toHaveValue('new@email.com'); - await expect(emailInput.locator('[data-test-email-address]')).toHaveCount(0); - await expect(page.locator('[data-test-notification-message="error"]')).toHaveText( - 'Error in saving email: An unknown error occurred while saving this email.', - ); - - user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); - await expect(user.email).toBe('old@email.com'); - await expect(user.emailVerified).toBe(true); - await expect(user.emailVerificationToken).toBe(null); - }); - - test.describe('Resend button', function () { - test('happy path', async ({ page, msw }) => { - let user = msw.db.user.create({ email: 'john@doe.com', emailVerificationToken: 'secret123' }); - await msw.authenticateAs(user); - - await page.goto('/settings/profile'); - await expect(page).toHaveURL('/settings/profile'); - const emailInput = page.locator('[data-test-email-input]'); - await expect(emailInput).toBeVisible(); - await expect(emailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); - await expect(emailInput.locator('[data-test-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-not-verified]')).toBeVisible(); - await expect(emailInput.locator('[data-test-verification-sent]')).toBeVisible(); - const button = emailInput.locator('[data-test-resend-button]'); - await expect(button).toBeEnabled(); - await expect(button).toHaveText('Resend'); - - await button.click(); - await expect(button).toBeDisabled(); - await expect(button).toHaveText('Sent!'); - }); - - test('server error', async ({ page, msw }) => { - let user = msw.db.user.create({ email: 'john@doe.com', emailVerificationToken: 'secret123' }); - await msw.authenticateAs(user); - - let error = HttpResponse.json({}, { status: 500 }); - await msw.worker.use(http.put('/api/v1/users/:user_id/resend', () => error)); - - await page.goto('/settings/profile'); - await expect(page).toHaveURL('/settings/profile'); - const emailInput = page.locator('[data-test-email-input]'); - await expect(emailInput).toBeVisible(); - await expect(emailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); - await expect(emailInput.locator('[data-test-verified]')).toHaveCount(0); - await expect(emailInput.locator('[data-test-not-verified]')).toBeVisible(); - await expect(emailInput.locator('[data-test-verification-sent]')).toBeVisible(); - const button = emailInput.locator('[data-test-resend-button]'); - await expect(button).toBeEnabled(); - await expect(button).toHaveText('Resend'); - - await button.click(); - await expect(button).toBeEnabled(); - await expect(button).toHaveText('Resend'); - await expect(page.locator('[data-test-notification-message="error"]')).toHaveText( - 'Unknown error in resending message', - ); - }); - }); -}); diff --git a/e2e/acceptance/email-confirmation.spec.ts b/e2e/acceptance/email-confirmation.spec.ts index 034a5d144dc..07c1cc28796 100644 --- a/e2e/acceptance/email-confirmation.spec.ts +++ b/e2e/acceptance/email-confirmation.spec.ts @@ -2,35 +2,37 @@ import { expect, test } from '@/e2e/helper'; test.describe('Acceptance | Email Confirmation', { tag: '@acceptance' }, () => { test('unauthenticated happy path', async ({ page, msw }) => { - let user = msw.db.user.create({ emailVerificationToken: 'badc0ffee' }); + let email = msw.db.email.create({ verified: false, token: 'badc0ffee' }); + let user = msw.db.user.create({ emails: [email] }); + await expect(email.verified).toBe(false); await page.goto('/confirm/badc0ffee'); - await expect(user.emailVerified).toBe(false); await expect(page).toHaveURL('/'); await expect(page.locator('[data-test-notification-message="success"]')).toBeVisible(); user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); - await expect(user.emailVerified).toBe(true); + await expect(user.emails[0].verified).toBe(true); }); test('authenticated happy path', async ({ page, msw, ember }) => { - let user = msw.db.user.create({ emailVerificationToken: 'badc0ffee' }); + let email = msw.db.email.create({ token: 'badc0ffee' }); + let user = msw.db.user.create({ emails: [email] }); await msw.authenticateAs(user); + await expect(email.verified).toBe(false); await page.goto('/confirm/badc0ffee'); - await expect(user.emailVerified).toBe(false); await expect(page).toHaveURL('/'); await expect(page.locator('[data-test-notification-message="success"]')).toBeVisible(); const emailVerified = await ember.evaluate(owner => { const { currentUser } = owner.lookup('service:session'); - return currentUser.email_verified; + return currentUser.emails[0].verified; }); expect(emailVerified).toBe(true); user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); - await expect(user.emailVerified).toBe(true); + await expect(user.emails[0].verified).toBe(true); }); test('error case', async ({ page }) => { diff --git a/e2e/acceptance/email.spec.ts b/e2e/acceptance/email.spec.ts new file mode 100644 index 00000000000..91f7a9203a3 --- /dev/null +++ b/e2e/acceptance/email.spec.ts @@ -0,0 +1,312 @@ +import { expect, test } from '@/e2e/helper'; +import { http, HttpResponse } from 'msw'; + +test.describe('Acceptance | Email Management', { tag: '@acceptance' }, () => { + test.describe('Add email', () => { + test('happy path', async ({ page, msw }) => { + let user = msw.db.user.create({ emails: [msw.db.email.create({ email: 'old@email.com', verified: true })] }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + await expect(page).toHaveURL('/settings/profile'); + + const existingEmail = page.locator('[data-test-email-input]:nth-of-type(1)'); + await expect(existingEmail.locator('[data-test-email-address]')).toContainText('old@email.com'); + await expect(existingEmail.locator('[data-test-verified]')).toBeVisible(); + await expect(existingEmail.locator('[data-test-unverified]')).toHaveCount(0); + await expect(existingEmail.locator('[data-test-verification-sent]')).toHaveCount(0); + await expect(existingEmail.locator('[data-test-resend-button]')).toHaveCount(0); + await expect(existingEmail.locator('[data-test-remove-button]')).toHaveCount(0); + + await expect(page.locator('[data-test-add-email-button]')).toBeVisible(); + await expect(page.locator('[data-test-add-email-input]')).not.toBeVisible(); + + await page.locator('[data-test-add-email-button]').click(); + + const addEmailForm = page.locator('[data-test-add-email-input]'); + const inputField = addEmailForm.locator('[data-test-input]'); + const submitButton = addEmailForm.locator('[data-test-save-button]'); + + await expect(addEmailForm).toBeVisible(); + await expect(addEmailForm.locator('[data-test-no-email]')).toHaveCount(0); + await expect(addEmailForm.locator('[data-test-unverified]')).toHaveCount(0); + await expect(addEmailForm.locator('[data-test-verified]')).toHaveCount(0); + await expect(addEmailForm.locator('[data-test-verification-sent]')).toHaveCount(0); + await expect(addEmailForm.locator('[data-test-resend-button]')).toHaveCount(0); + await expect(inputField).toContainText(''); + await expect(submitButton).toBeDisabled(); + + await inputField.fill(''); + await expect(inputField).toHaveValue(''); + await expect(submitButton).toBeDisabled(); + + await inputField.fill('notanemail'); + await expect(inputField).toHaveValue('notanemail'); + await expect(submitButton).toBeDisabled(); + + await inputField.fill('new@email.com'); + await expect(inputField).toHaveValue('new@email.com'); + await expect(submitButton).toBeEnabled(); + + await submitButton.click(); + const createdEmail = page.locator('[data-test-email-input]:nth-of-type(2)'); + await expect(createdEmail.locator('[data-test-email-address]')).toContainText('new@email.com'); + await expect(createdEmail.locator('[data-test-verified]')).toHaveCount(0); + await expect(createdEmail.locator('[data-test-unverified]')).toHaveCount(0); + await expect(createdEmail.locator('[data-test-verification-sent]')).toBeVisible(); + await expect(createdEmail.locator('[data-test-resend-button]')).toBeEnabled(); + + user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); + await expect(user.emails.length).toBe(2); + await expect(user.emails[0].email).toBe('old@email.com'); + await expect(user.emails[1].email).toBe('new@email.com'); + await expect(user.emails[1].verified).toBe(false); + }); + + test('happy path with no previous emails', async ({ page, msw }) => { + let user = msw.db.user.create({ emails: [] }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + await expect(page).toHaveURL('/settings/profile'); + + const addEmailButton = page.locator('[data-test-add-email-button]'); + const addEmailForm = page.locator('[data-test-add-email-input]'); + const addEmailInput = addEmailForm.locator('[data-test-input]'); + + await expect(page.locator('[data-test-email-input]')).toHaveCount(0); + await expect(page.locator('[data-test-add-email-input]')).toHaveCount(0); + await expect(addEmailButton).toBeVisible(); + + await addEmailButton.click(); + await expect(addEmailForm).toBeVisible(); + await expect(addEmailForm.locator('[data-test-input]')).toContainText(''); + await addEmailInput.fill('new@email.com'); + await expect(addEmailInput).toHaveValue('new@email.com'); + await addEmailForm.locator('[data-test-save-button]').click(); + + const createdEmail = page.locator('[data-test-email-input]:nth-of-type(1)'); + await expect(createdEmail.locator('[data-test-email-address]')).toContainText('new@email.com'); + await expect(createdEmail.locator('[data-test-verified]')).toHaveCount(0); + await expect(createdEmail.locator('[data-test-unverified]')).toHaveCount(0); + await expect(createdEmail.locator('[data-test-verification-sent]')).toBeVisible(); + await expect(createdEmail.locator('[data-test-resend-button]')).toBeEnabled(); + + user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); + await expect(user.emails.length).toBe(1); + await expect(user.emails[0].email).toBe('new@email.com'); + await expect(user.emails[0].verified).toBe(false); + }); + + test('server error', async ({ page, msw }) => { + let user = msw.db.user.create({ emails: [] }); + await msw.authenticateAs(user); + + let error = HttpResponse.json({}, { status: 500 }); + await msw.worker.use(http.post('/api/v1/users/:user_id/emails', () => error)); + + await page.goto('/settings/profile'); + + const addEmailForm = page.locator('[data-test-add-email-input]'); + + await page.locator('[data-test-add-email-button]').click(); + await addEmailForm.locator('[data-test-input]').fill('new@email.com'); + await addEmailForm.locator('[data-test-save-button]').click(); + + await expect(page.locator('[data-test-email-input]')).toHaveCount(0); + await expect(page.locator('[data-test-notification-message="error"]')).toHaveText( + 'Unknown error in saving email', + ); + + user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); + await expect(user.emails.length).toBe(0); + }); + }); + + test.describe('Remove email', () => { + test('happy path', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [msw.db.email.create({ email: 'john@doe.com' }), msw.db.email.create({ email: 'jane@doe.com' })], + }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + await expect(page).toHaveURL('/settings/profile'); + const emailInputs = page.locator('[data-test-email-input]'); + + await expect(emailInputs).toHaveCount(2); + const firstEmailInput = emailInputs.nth(0); + const secondEmailInput = emailInputs.nth(1); + + await expect(firstEmailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + await expect(secondEmailInput.locator('[data-test-email-address]')).toContainText('jane@doe.com'); + + await expect(firstEmailInput.locator('[data-test-remove-button]')).toBeVisible(); + await expect(secondEmailInput.locator('[data-test-remove-button]')).toBeVisible(); + + await secondEmailInput.locator('[data-test-remove-button]').click(); + await expect(emailInputs).toHaveCount(1); + await expect(firstEmailInput.locator('[data-test-remove-button]')).toHaveCount(0); + + user = msw.db.user.findFirst({ where: { id: { equals: user.id } } }); + await expect(user.emails.length).toBe(1); + await expect(user.emails[0].email).toBe('john@doe.com'); + }); + + test('cannot remove primary email', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [ + msw.db.email.create({ email: 'primary@doe.com', primary: true }), + msw.db.email.create({ email: 'john@doe.com' }), + ], + }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + await expect(page).toHaveURL('/settings/profile'); + + const emailInputs = page.locator('[data-test-email-input]'); + await expect(emailInputs).toHaveCount(2); + const primaryEmailInput = emailInputs.nth(0); + const johnEmailInput = emailInputs.nth(1); + + await expect(primaryEmailInput.locator('[data-test-email-address]')).toContainText('primary@doe.com'); + await expect(johnEmailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + + await expect(primaryEmailInput.locator('[data-test-remove-button]')).toBeDisabled(); + await expect(primaryEmailInput.locator('[data-test-remove-button]')).toHaveAttribute( + 'title', + 'Cannot delete primary email', + ); + await expect(johnEmailInput.locator('[data-test-remove-button]')).toBeVisible(); + }); + + test('no delete button when only one email', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [ + msw.db.email.create({ + email: 'john@doe.com', + }), + ], + }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + await expect(page).toHaveURL('/settings/profile'); + const emailInput = page.locator('[data-test-email-input]'); + await expect(emailInput).toBeVisible(); + await expect(emailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + await expect(emailInput.locator('[data-test-remove-button]')).toHaveCount(0); + }); + + test('server error', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [msw.db.email.create({ email: 'john@doe.com' }), msw.db.email.create({ email: 'jane@doe.com' })], + }); + await msw.authenticateAs(user); + + let error = HttpResponse.json({}, { status: 500 }); + await msw.worker.use(http.delete('/api/v1/users/:user_id/emails/:email_id', () => error)); + + await page.goto('/settings/profile'); + + const emailInputs = page.locator('[data-test-email-input]'); + await expect(emailInputs).toHaveCount(2); + const johnEmailInput = emailInputs.nth(0); + await expect(johnEmailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + await expect(johnEmailInput.locator('[data-test-remove-button]')).toBeEnabled(); + await johnEmailInput.locator('[data-test-remove-button]').click(); + await expect(page.locator('[data-test-notification-message="error"]')).toHaveText( + 'Unknown error in deleting email', + ); + await expect(johnEmailInput.locator('[data-test-remove-button]')).toBeEnabled(); + }); + }); + + test.describe('Resend verification email', function () { + test('happy path', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [msw.db.email.create({ email: 'john@doe.com', verified: false, verification_email_sent: true })], + }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + + const emailInput = page.locator('[data-test-email-input]:nth-of-type(1)'); + await expect(emailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + + const resendButton = emailInput.locator('[data-test-resend-button]'); + await expect(resendButton).toBeEnabled(); + await expect(resendButton).toContainText('Resend'); + await expect(emailInput.locator('[data-test-verified]')).toHaveCount(0); + await expect(emailInput.locator('[data-test-unverified]')).toHaveCount(0); + await expect(emailInput.locator('[data-test-verification-sent]')).toBeVisible(); + + await resendButton.click(); + await expect(emailInput.locator('[data-test-verification-sent]')).toBeVisible(); + await expect(resendButton).toContainText('Sent!'); + await expect(resendButton).toBeDisabled(); + }); + + test('server error', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [msw.db.email.create({ email: 'john@doe.com', verified: false, verification_email_sent: true })], + }); + await msw.authenticateAs(user); + + let error = HttpResponse.json({}, { status: 500 }); + await msw.worker.use(http.put('/api/v1/users/:user_id/emails/:email_id/resend', () => error)); + + await page.goto('/settings/profile'); + + const emailInput = page.locator('[data-test-email-input]:nth-of-type(1)'); + await expect(emailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + + const resendButton = emailInput.locator('[data-test-resend-button]'); + await expect(resendButton).toBeEnabled(); + + await resendButton.click(); + await expect(page.locator('[data-test-notification-message="error"]')).toHaveText( + 'Unknown error in resending message', + ); + await expect(resendButton).toBeEnabled(); + }); + }); + + test.describe('Switch primary email', () => { + test('happy path', async ({ page, msw }) => { + let user = msw.db.user.create({ + emails: [ + msw.db.email.create({ email: 'john@doe.com', verified: true, primary: true }), + msw.db.email.create({ email: 'jane@doe.com', verified: true, primary: false }), + ], + }); + await msw.authenticateAs(user); + + await page.goto('/settings/profile'); + + const emailInputs = page.locator('[data-test-email-input]'); + await expect(emailInputs).toHaveCount(2); + + const johnEmailInput = emailInputs.nth(0); + const janeEmailInput = emailInputs.nth(1); + + await expect(johnEmailInput.locator('[data-test-email-address]')).toContainText('john@doe.com'); + await expect(janeEmailInput.locator('[data-test-email-address]')).toContainText('jane@doe.com'); + + const johnMarkPrimaryButton = johnEmailInput.locator('[data-test-primary-button]'); + const janeMarkPrimaryButton = janeEmailInput.locator('[data-test-primary-button]'); + + await expect(johnEmailInput.locator('[data-test-primary]')).toBeVisible(); + await expect(janeEmailInput.locator('[data-test-primary]')).toHaveCount(0); + await expect(johnMarkPrimaryButton).toHaveCount(0); + await expect(janeMarkPrimaryButton).toBeEnabled(); + + await janeMarkPrimaryButton.click(); + await expect(johnEmailInput.locator('[data-test-primary]')).toHaveCount(0); + await expect(janeEmailInput.locator('[data-test-primary]')).toBeVisible(); + await expect(johnMarkPrimaryButton).toBeEnabled(); + await expect(janeMarkPrimaryButton).toHaveCount(0); + }); + }); +}); diff --git a/e2e/acceptance/publish-notifications.spec.ts b/e2e/acceptance/publish-notifications.spec.ts index 2d801fff89c..2f8f5ceab32 100644 --- a/e2e/acceptance/publish-notifications.spec.ts +++ b/e2e/acceptance/publish-notifications.spec.ts @@ -4,7 +4,7 @@ import { http, HttpResponse } from 'msw'; test.describe('Acceptance | publish notifications', { tag: '@acceptance' }, () => { test('unsubscribe and resubscribe', async ({ page, msw }) => { - let user = msw.db.user.create(); + let user = msw.db.user.create({ emails: [msw.db.email.create()] }); await msw.authenticateAs(user); await page.goto('/settings/profile'); @@ -27,7 +27,7 @@ test.describe('Acceptance | publish notifications', { tag: '@acceptance' }, () = }); test('loading state', async ({ page, msw }) => { - let user = msw.db.user.create(); + let user = msw.db.user.create({ emails: [msw.db.email.create()] }); await msw.authenticateAs(user); let deferred = defer(); @@ -48,7 +48,7 @@ test.describe('Acceptance | publish notifications', { tag: '@acceptance' }, () = }); test('error state', async ({ page, msw }) => { - let user = msw.db.user.create(); + let user = msw.db.user.create({ emails: [msw.db.email.create()] }); await msw.authenticateAs(user); msw.worker.use(http.put('/api/v1/users/:user_id', () => HttpResponse.text('', { status: 500 }))); diff --git a/e2e/acceptance/settings/settings.spec.ts b/e2e/acceptance/settings/settings.spec.ts index 9adf2c31ea1..366d16d99c4 100644 --- a/e2e/acceptance/settings/settings.spec.ts +++ b/e2e/acceptance/settings/settings.spec.ts @@ -2,7 +2,10 @@ import { expect, test } from '@/e2e/helper'; test.describe('Acceptance | Settings', { tag: '@acceptance' }, () => { test.beforeEach(async ({ msw }) => { - let user1 = msw.db.user.create({ name: 'blabaere' }); + let user1 = msw.db.user.create({ + name: 'blabaere', + emails: [msw.db.email.create({ email: 'blabaere@crates.io', primary: true })], + }); let user2 = msw.db.user.create({ name: 'thehydroimpulse' }); let team1 = msw.db.team.create({ org: 'org', name: 'blabaere' }); let team2 = msw.db.team.create({ org: 'org', name: 'thehydroimpulse' }); diff --git a/e2e/routes/crate/settings.spec.ts b/e2e/routes/crate/settings.spec.ts index 2eba1136335..177cb8bc3b0 100644 --- a/e2e/routes/crate/settings.spec.ts +++ b/e2e/routes/crate/settings.spec.ts @@ -4,7 +4,9 @@ import { http, HttpResponse } from 'msw'; test.describe('Route | crate.settings', { tag: '@routes' }, () => { async function prepare(msw) { - let user = msw.db.user.create(); + let user = msw.db.user.create({ + emails: [msw.db.email.create({ email: 'user-1@crates.io', primary: true, verified: true })], + }); let crate = msw.db.crate.create({ name: 'foo' }); msw.db.version.create({ crate }); diff --git a/e2e/routes/crate/settings/new-trusted-publisher.spec.ts b/e2e/routes/crate/settings/new-trusted-publisher.spec.ts index d69fcf81b0d..cbf0b11cd8d 100644 --- a/e2e/routes/crate/settings/new-trusted-publisher.spec.ts +++ b/e2e/routes/crate/settings/new-trusted-publisher.spec.ts @@ -4,7 +4,9 @@ import { defer } from '@/e2e/deferred'; test.describe('Route | crate.settings.new-trusted-publisher', { tag: '@routes' }, () => { async function prepare(msw) { - let user = msw.db.user.create(); + let user = msw.db.user.create({ + emails: [msw.db.email.create({ email: 'user-1@crates.io', primary: true, verified: true })], + }); let crate = msw.db.crate.create({ name: 'foo' }); msw.db.version.create({ crate }); 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..abb156918f3 --- /dev/null +++ b/migrations/2025-07-22-091706_multiple_emails/down.sql @@ -0,0 +1,37 @@ +-- 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/packages/crates-io-msw/handlers/emails/add.js b/packages/crates-io-msw/handlers/emails/add.js new file mode 100644 index 00000000000..c76eccde591 --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/add.js @@ -0,0 +1,36 @@ +import { http, HttpResponse } from 'msw'; + +import { db } from '../../index.js'; +import { serializeEmail } from '../../serializers/email.js'; +import { getSession } from '../../utils/session.js'; + +export default http.post('/api/v1/users/:user_id/emails', async ({ params, request }) => { + let { user_id } = params; + + let { user } = getSession(); + if (!user) { + return HttpResponse.json({ errors: [{ detail: 'must be logged in to perform that action' }] }, { status: 403 }); + } + if (user.id.toString() !== user_id) { + return HttpResponse.json({ errors: [{ detail: 'current user does not match requested user' }] }, { status: 400 }); + } + + if (!user) { + return HttpResponse.json({ errors: [{ detail: 'User not found.' }] }, { status: 404 }); + } + + let email = db.email.create({ + email: (await request.json()).email, + verified: false, + verification_email_sent: true, + primary: false, + }); + db.user.update({ + where: { id: { equals: user.id } }, + data: { + emails: [...user.emails, email], + }, + }); + + return HttpResponse.json(serializeEmail(email)); +}); diff --git a/packages/crates-io-msw/handlers/emails/add.test.js b/packages/crates-io-msw/handlers/emails/add.test.js new file mode 100644 index 00000000000..9c5f5486866 --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/add.test.js @@ -0,0 +1,39 @@ +import { assert, test } from 'vitest'; + +import { db } from '../../index.js'; + +test('returns an error for unauthenticated requests', async function () { + let response = await fetch('/api/v1/users/1/emails', { method: 'POST' }); + assert.strictEqual(response.status, 403); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'must be logged in to perform that action' }], + }); +}); + +test('returns an error for requests to a different user', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let response = await fetch('/api/v1/users/512/emails', { method: 'POST' }); + assert.strictEqual(response.status, 400); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'current user does not match requested user' }], + }); +}); + +test('returns email for valid, authenticated request', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let response = await fetch(`/api/v1/users/${user.id}/emails`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: 'test@example.com' }), + }); + assert.strictEqual(response.status, 200); + let email = await response.json(); + assert.strictEqual(email.email, 'test@example.com'); + assert.strictEqual(email.verified, false); + assert.strictEqual(email.verification_email_sent, true); + assert.strictEqual(email.primary, false); +}); diff --git a/packages/crates-io-msw/handlers/emails/confirm.js b/packages/crates-io-msw/handlers/emails/confirm.js new file mode 100644 index 00000000000..824b58c9314 --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/confirm.js @@ -0,0 +1,23 @@ +import { http, HttpResponse } from 'msw'; + +import { db } from '../../index.js'; +import { serializeEmail } from '../../serializers/email.js'; + +export default http.put('/api/v1/confirm/:token', ({ params }) => { + let { token } = params; + + let email = db.email.findFirst({ where: { token: { equals: token } } }); + if (!email) { + return HttpResponse.json({ errors: [{ detail: 'Email belonging to token not found.' }] }, { status: 400 }); + } + + db.email.update({ where: { id: email.id }, data: { verified: true } }); + + return HttpResponse.json({ + ok: true, + email: serializeEmail({ + ...email, + verified: true, + }), + }); +}); diff --git a/packages/crates-io-msw/handlers/users/confirm-email.test.js b/packages/crates-io-msw/handlers/emails/confirm.test.js similarity index 51% rename from packages/crates-io-msw/handlers/users/confirm-email.test.js rename to packages/crates-io-msw/handlers/emails/confirm.test.js index 64c7fea7207..e86c4d23374 100644 --- a/packages/crates-io-msw/handlers/users/confirm-email.test.js +++ b/packages/crates-io-msw/handlers/emails/confirm.test.js @@ -1,31 +1,34 @@ import { assert, test } from 'vitest'; import { db } from '../../index.js'; +import { serializeEmail } from '../../serializers/email.js'; test('returns `ok: true` for a known token (unauthenticated)', async function () { - let user = db.user.create({ emailVerificationToken: 'foo' }); - assert.strictEqual(user.emailVerified, false); + let email = db.email.create({ token: 'foo' }); + let user = db.user.create({ emails: [email] }); + assert.strictEqual(email.verified, false); let response = await fetch('/api/v1/confirm/foo', { method: 'PUT' }); assert.strictEqual(response.status, 200); - assert.deepEqual(await response.json(), { ok: true }); + assert.deepEqual(await response.json(), { ok: true, email: serializeEmail({ ...email, verified: true }) }); - user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.emailVerified, true); + email = db.email.findFirst({ where: { id: user.emails[0].id } }); + assert.strictEqual(email.verified, true); }); test('returns `ok: true` for a known token (authenticated)', async function () { - let user = db.user.create({ emailVerificationToken: 'foo' }); - assert.strictEqual(user.emailVerified, false); + let email = db.email.create({ token: 'foo' }); + let user = db.user.create({ emails: [email] }); + assert.strictEqual(email.verified, false); db.mswSession.create({ user }); let response = await fetch('/api/v1/confirm/foo', { method: 'PUT' }); assert.strictEqual(response.status, 200); - assert.deepEqual(await response.json(), { ok: true }); + assert.deepEqual(await response.json(), { ok: true, email: serializeEmail({ ...email, verified: true }) }); - user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.emailVerified, true); + email = db.email.findFirst({ where: { id: user.emails[0].id } }); + assert.strictEqual(email.verified, true); }); test('returns an error for unknown tokens', async function () { diff --git a/packages/crates-io-msw/handlers/emails/delete.js b/packages/crates-io-msw/handlers/emails/delete.js new file mode 100644 index 00000000000..c1398dcf1ac --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/delete.js @@ -0,0 +1,39 @@ +import { http, HttpResponse } from 'msw'; + +import { db } from '../../index.js'; +import { getSession } from '../../utils/session.js'; + +export default http.delete('/api/v1/users/:user_id/emails/:email_id', ({ params }) => { + let { user_id, email_id } = params; + + let { user } = getSession(); + if (!user) { + return HttpResponse.json({ errors: [{ detail: 'must be logged in to perform that action' }] }, { status: 403 }); + } + if (user.id.toString() !== user_id) { + return HttpResponse.json({ errors: [{ detail: 'current user does not match requested user' }] }, { status: 400 }); + } + + let email = db.email.findFirst({ where: { id: { equals: parseInt(email_id) } } }); + if (!email) { + return HttpResponse.json({ errors: [{ detail: 'Email not found.' }] }, { status: 404 }); + } + + // Prevent deletion if this is primary email + if (email.primary) { + return HttpResponse.json( + { errors: [{ detail: 'cannot delete primary email, please set another email as primary first' }] }, + { status: 400 }, + ); + } + + // Check how many emails the user has, if this is the only verified email, prevent deletion + let userEmails = db.email.findMany({ where: { user_id: { equals: user.id } } }); + if (userEmails.length === 1) { + return HttpResponse.json({ errors: [{ detail: 'Cannot delete your only email address.' }] }, { status: 400 }); + } + + db.email.delete({ where: { id: { equals: parseInt(email_id) } } }); + + return HttpResponse.json({ ok: true }); +}); diff --git a/packages/crates-io-msw/handlers/emails/delete.test.js b/packages/crates-io-msw/handlers/emails/delete.test.js new file mode 100644 index 00000000000..ae5e4c26697 --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/delete.test.js @@ -0,0 +1,66 @@ +import { assert, test } from 'vitest'; + +import { db } from '../../index.js'; + +test('returns an error for unauthenticated requests', async function () { + let response = await fetch('/api/v1/users/1/emails/1', { method: 'DELETE' }); + assert.strictEqual(response.status, 403); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'must be logged in to perform that action' }], + }); +}); + +test('returns an error for requests to a different user', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let response = await fetch('/api/v1/users/512/emails/1', { method: 'DELETE' }); + assert.strictEqual(response.status, 400); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'current user does not match requested user' }], + }); +}); + +test('returns an error for non-existent email', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let response = await fetch(`/api/v1/users/${user.id}/emails/999`, { method: 'DELETE' }); + assert.strictEqual(response.status, 404); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'Email not found.' }], + }); +}); + +test('prevents deletion of primary email', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let email = db.email.create({ user_id: user.id, email: 'test@example.com', primary: true }); + + let response = await fetch(`/api/v1/users/${user.id}/emails/${email.id}`, { method: 'DELETE' }); + assert.strictEqual(response.status, 400); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'cannot delete primary email, please set another email as primary first' }], + }); +}); + +test('successfully deletes alternate email', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let email1 = db.email.create({ user_id: user.id, email: 'test1@example.com', primary: true }); + let email2 = db.email.create({ user_id: user.id, email: 'test2@example.com' }); + + let response = await fetch(`/api/v1/users/${user.id}/emails/${email2.id}`, { method: 'DELETE' }); + assert.strictEqual(response.status, 200); + assert.deepEqual(await response.json(), { ok: true }); + + // Check that email2 was deleted + let deletedEmail = db.email.findFirst({ where: { id: { equals: email2.id } } }); + assert.strictEqual(deletedEmail, null); + + // Check that email1 still exists + let remainingEmail = db.email.findFirst({ where: { id: { equals: email1.id } } }); + assert.strictEqual(remainingEmail.email, 'test1@example.com'); +}); diff --git a/packages/crates-io-msw/handlers/users/resend.js b/packages/crates-io-msw/handlers/emails/resend.js similarity index 61% rename from packages/crates-io-msw/handlers/users/resend.js rename to packages/crates-io-msw/handlers/emails/resend.js index a9afc9395b3..4ca720edb15 100644 --- a/packages/crates-io-msw/handlers/users/resend.js +++ b/packages/crates-io-msw/handlers/emails/resend.js @@ -1,8 +1,9 @@ import { http, HttpResponse } from 'msw'; +import { db } from '../../index.js'; import { getSession } from '../../utils/session.js'; -export default http.put('/api/v1/users/:user_id/resend', ({ params }) => { +export default http.put('/api/v1/users/:user_id/emails/:email_id/resend', ({ params }) => { let { user } = getSession(); if (!user) { return HttpResponse.json({ errors: [{ detail: 'must be logged in to perform that action' }] }, { status: 403 }); @@ -12,6 +13,11 @@ export default http.put('/api/v1/users/:user_id/resend', ({ params }) => { return HttpResponse.json({ errors: [{ detail: 'current user does not match requested user' }] }, { status: 400 }); } + let email = db.email.findFirst({ where: { id: { equals: parseInt(params.email_id) } } }); + if (!email) { + return HttpResponse.json({ errors: [{ detail: 'Email not found.' }] }, { status: 404 }); + } + // let's pretend that we're sending an email here... :D return HttpResponse.json({ ok: true }); diff --git a/packages/crates-io-msw/handlers/users/resend.test.js b/packages/crates-io-msw/handlers/emails/resend.test.js similarity index 56% rename from packages/crates-io-msw/handlers/users/resend.test.js rename to packages/crates-io-msw/handlers/emails/resend.test.js index a260624ff5a..1d52b632743 100644 --- a/packages/crates-io-msw/handlers/users/resend.test.js +++ b/packages/crates-io-msw/handlers/emails/resend.test.js @@ -3,27 +3,27 @@ import { assert, test } from 'vitest'; import { db } from '../../index.js'; test('returns `ok`', async function () { - let user = db.user.create(); + let user = db.user.create({ emails: [db.email.create({ verified: false })] }); db.mswSession.create({ user }); - let response = await fetch(`/api/v1/users/${user.id}/resend`, { method: 'PUT' }); + let response = await fetch(`/api/v1/users/${user.id}/emails/${user.emails[0].id}/resend`, { method: 'PUT' }); assert.strictEqual(response.status, 200); assert.deepEqual(await response.json(), { ok: true }); }); test('returns 403 when not logged in', async function () { - let user = db.user.create(); + let user = db.user.create({ emails: [db.email.create({ verified: false })] }); - let response = await fetch(`/api/v1/users/${user.id}/resend`, { method: 'PUT' }); + let response = await fetch(`/api/v1/users/${user.id}/emails/${user.emails[0].id}/resend`, { method: 'PUT' }); assert.strictEqual(response.status, 403); assert.deepEqual(await response.json(), { errors: [{ detail: 'must be logged in to perform that action' }] }); }); test('returns 400 when requesting the wrong user id', async function () { - let user = db.user.create(); + let user = db.user.create({ emails: [db.email.create({ verified: false })] }); db.mswSession.create({ user }); - let response = await fetch(`/api/v1/users/wrong-id/resend`, { method: 'PUT' }); + let response = await fetch(`/api/v1/users/wrong-id/emails/${user.emails[0].id}/resend`, { method: 'PUT' }); assert.strictEqual(response.status, 400); assert.deepEqual(await response.json(), { errors: [{ detail: 'current user does not match requested user' }] }); }); diff --git a/packages/crates-io-msw/handlers/emails/set-primary.js b/packages/crates-io-msw/handlers/emails/set-primary.js new file mode 100644 index 00000000000..667433a878c --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/set-primary.js @@ -0,0 +1,36 @@ +import { http, HttpResponse } from 'msw'; + +import { db } from '../../index.js'; +import { getSession } from '../../utils/session.js'; + +export default http.put('/api/v1/users/:user_id/emails/:email_id/set_primary', async ({ params }) => { + let { user_id, email_id } = params; + + let { user } = getSession(); + if (!user) { + return HttpResponse.json({ errors: [{ detail: 'must be logged in to perform that action' }] }, { status: 403 }); + } + if (user.id.toString() !== user_id) { + return HttpResponse.json({ errors: [{ detail: 'current user does not match requested user' }] }, { status: 400 }); + } + + let email = db.email.findFirst({ where: { id: { equals: parseInt(email_id) } } }); + if (!email) { + return HttpResponse.json({ errors: [{ detail: 'Email not found.' }] }, { status: 404 }); + } + + // Update email to set as primary + db.email.update({ + where: { id: { equals: parseInt(email_id) } }, + data: { primary: true }, + }); + // Update all other emails to remove primary status + db.email.updateMany({ + where: { user_id: { equals: user.id }, id: { notEquals: parseInt(email_id) } }, + data: { primary: false }, + }); + + let updatedEmail = db.email.findFirst({ where: { id: { equals: parseInt(email_id) } } }); + + return HttpResponse.json(updatedEmail); +}); diff --git a/packages/crates-io-msw/handlers/emails/set-primary.test.js b/packages/crates-io-msw/handlers/emails/set-primary.test.js new file mode 100644 index 00000000000..40c923d18f6 --- /dev/null +++ b/packages/crates-io-msw/handlers/emails/set-primary.test.js @@ -0,0 +1,50 @@ +import { assert, test } from 'vitest'; + +import { db } from '../../index.js'; + +test('returns an error for unauthenticated requests', async function () { + let response = await fetch('/api/v1/users/1/emails/1/set_primary', { method: 'PUT' }); + assert.strictEqual(response.status, 403); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'must be logged in to perform that action' }], + }); +}); + +test('returns an error for requests to a different user', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let response = await fetch('/api/v1/users/512/emails/1/set_primary', { method: 'PUT' }); + assert.strictEqual(response.status, 400); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'current user does not match requested user' }], + }); +}); + +test('returns an error for non-existent email', async function () { + let user = db.user.create(); + db.mswSession.create({ user }); + + let response = await fetch(`/api/v1/users/${user.id}/emails/999/set_primary`, { method: 'PUT' }); + assert.strictEqual(response.status, 404); + assert.deepEqual(await response.json(), { + errors: [{ detail: 'Email not found.' }], + }); +}); + +test('successfully marks email as primary', async function () { + let email = db.email.create({ primary: false }); + let user = db.user.create({ emails: [email] }); + + db.mswSession.create({ user }); + + let response = await fetch(`/api/v1/users/${user.id}/emails/${email.id}/set_primary`, { method: 'PUT' }); + assert.strictEqual(response.status, 200); + let updatedEmail = await response.json(); + assert.strictEqual(updatedEmail.primary, true); + assert.strictEqual(updatedEmail.email, 'foo@crates.io'); + + // Verify the change was persisted + let emailFromDb = db.email.findFirst({ where: { id: { equals: email.id } } }); + assert.strictEqual(emailFromDb.primary, true); +}); diff --git a/packages/crates-io-msw/handlers/trustpub/github-configs/create.js b/packages/crates-io-msw/handlers/trustpub/github-configs/create.js index 1e6a7e02a23..04a37f120e1 100644 --- a/packages/crates-io-msw/handlers/trustpub/github-configs/create.js +++ b/packages/crates-io-msw/handlers/trustpub/github-configs/create.js @@ -38,7 +38,7 @@ export default http.post('/api/v1/trusted_publishing/github_configs', async ({ r } // Check if the user has a verified email - let hasVerifiedEmail = user.emailVerified; + let hasVerifiedEmail = user.emails.some(email => email.verified); if (!hasVerifiedEmail) { let detail = 'You must verify your email address to create a Trusted Publishing config'; return HttpResponse.json({ errors: [{ detail }] }, { status: 403 }); diff --git a/packages/crates-io-msw/handlers/trustpub/github-configs/create.test.js b/packages/crates-io-msw/handlers/trustpub/github-configs/create.test.js index e93205fad31..586bb2c6372 100644 --- a/packages/crates-io-msw/handlers/trustpub/github-configs/create.test.js +++ b/packages/crates-io-msw/handlers/trustpub/github-configs/create.test.js @@ -16,7 +16,7 @@ test('happy path', async function () { let crate = db.crate.create({ name: 'test-crate' }); db.version.create({ crate }); - let user = db.user.create({ emailVerified: true }); + let user = db.user.create({ emails: [db.email.create({ verified: true })] }); db.mswSession.create({ user }); // Create crate ownership @@ -58,7 +58,7 @@ test('happy path with environment', async function () { let crate = db.crate.create({ name: 'test-crate-env' }); db.version.create({ crate }); - let user = db.user.create({ emailVerified: true }); + let user = db.user.create({ emails: [db.email.create({ verified: true })] }); db.mswSession.create({ user }); // Create crate ownership @@ -199,7 +199,7 @@ test('returns 403 if user email is not verified', async function () { let crate = db.crate.create({ name: 'test-crate-unverified' }); db.version.create({ crate }); - let user = db.user.create({ emailVerified: false }); + let user = db.user.create({ emails: [db.email.create({ verified: false })] }); db.mswSession.create({ user }); // Create crate ownership diff --git a/packages/crates-io-msw/handlers/trustpub/github-configs/delete.test.js b/packages/crates-io-msw/handlers/trustpub/github-configs/delete.test.js index a4e09990b16..c83781fca0e 100644 --- a/packages/crates-io-msw/handlers/trustpub/github-configs/delete.test.js +++ b/packages/crates-io-msw/handlers/trustpub/github-configs/delete.test.js @@ -6,7 +6,7 @@ test('happy path', async function () { let crate = db.crate.create({ name: 'test-crate' }); db.version.create({ crate }); - let user = db.user.create({ email_verified: true }); + let user = db.user.create({ emails: [db.email.create({ verified: true })] }); db.mswSession.create({ user }); // Create crate ownership diff --git a/packages/crates-io-msw/handlers/trustpub/github-configs/list.test.js b/packages/crates-io-msw/handlers/trustpub/github-configs/list.test.js index b7394b5a20d..21ad5d554c9 100644 --- a/packages/crates-io-msw/handlers/trustpub/github-configs/list.test.js +++ b/packages/crates-io-msw/handlers/trustpub/github-configs/list.test.js @@ -6,7 +6,7 @@ test('happy path', async function () { let crate = db.crate.create({ name: 'test-crate' }); db.version.create({ crate }); - let user = db.user.create({ email_verified: true }); + let user = db.user.create({ emails: [db.email.create({ verified: true })] }); db.mswSession.create({ user }); // Create crate ownership @@ -67,7 +67,7 @@ test('happy path with no configs', async function () { let crate = db.crate.create({ name: 'test-crate-empty' }); db.version.create({ crate }); - let user = db.user.create({ email_verified: true }); + let user = db.user.create({ emails: [db.email.create({ verified: true })] }); db.mswSession.create({ user }); // Create crate ownership diff --git a/packages/crates-io-msw/handlers/users.js b/packages/crates-io-msw/handlers/users.js index 51a9a9e05b6..922d802e1d7 100644 --- a/packages/crates-io-msw/handlers/users.js +++ b/packages/crates-io-msw/handlers/users.js @@ -1,7 +1,10 @@ -import confirmEmail from './users/confirm-email.js'; +import addEmail from './emails/add.js'; +import confirmEmail from './emails/confirm.js'; +import deleteEmail from './emails/delete.js'; +import resend from './emails/resend.js'; +import enableNotifications from './emails/set-primary.js'; import getUser from './users/get.js'; import me from './users/me.js'; -import resend from './users/resend.js'; import updateUser from './users/update.js'; -export default [getUser, updateUser, resend, me, confirmEmail]; +export default [getUser, updateUser, resend, me, confirmEmail, addEmail, deleteEmail, enableNotifications]; diff --git a/packages/crates-io-msw/handlers/users/confirm-email.js b/packages/crates-io-msw/handlers/users/confirm-email.js deleted file mode 100644 index d42c7b13d44..00000000000 --- a/packages/crates-io-msw/handlers/users/confirm-email.js +++ /dev/null @@ -1,16 +0,0 @@ -import { http, HttpResponse } from 'msw'; - -import { db } from '../../index.js'; - -export default http.put('/api/v1/confirm/:token', ({ params }) => { - let { token } = params; - - let user = db.user.findFirst({ where: { emailVerificationToken: { equals: token } } }); - if (!user) { - return HttpResponse.json({ errors: [{ detail: 'Email belonging to token not found.' }] }, { status: 400 }); - } - - db.user.update({ where: { id: user.id }, data: { emailVerified: true, emailVerificationToken: null } }); - - return HttpResponse.json({ ok: true }); -}); diff --git a/packages/crates-io-msw/handlers/users/me.test.js b/packages/crates-io-msw/handlers/users/me.test.js index fba39926019..a8d77dd445c 100644 --- a/packages/crates-io-msw/handlers/users/me.test.js +++ b/packages/crates-io-msw/handlers/users/me.test.js @@ -3,7 +3,16 @@ import { assert, test } from 'vitest'; import { db } from '../../index.js'; test('returns the `user` resource including the private fields', async function () { - let user = db.user.create(); + let user = db.user.create({ + emails: [ + db.email.create({ + email: 'user-1@crates.io', + primary: true, + verification_email_sent: true, + verified: true, + }), + ], + }); db.mswSession.create({ user }); let response = await fetch('/api/v1/me'); @@ -12,9 +21,15 @@ test('returns the `user` resource including the private fields', async function user: { id: 1, avatar: 'https://avatars1.githubusercontent.com/u/14631425?v=4', - email: 'user-1@crates.io', - email_verification_sent: true, - email_verified: true, + emails: [ + { + id: 1, + email: 'user-1@crates.io', + verified: true, + verification_email_sent: true, + primary: true, + }, + ], is_admin: false, login: 'user-1', name: 'User 1', diff --git a/packages/crates-io-msw/handlers/users/update.js b/packages/crates-io-msw/handlers/users/update.js index 83480ee544a..f8f570fd744 100644 --- a/packages/crates-io-msw/handlers/users/update.js +++ b/packages/crates-io-msw/handlers/users/update.js @@ -25,20 +25,5 @@ export default http.put('/api/v1/users/:user_id', async ({ params, request }) => }); } - if (json.user.email !== undefined) { - if (!json.user.email) { - return HttpResponse.json({ errors: [{ detail: 'empty email rejected' }] }, { status: 400 }); - } - - db.user.update({ - where: { id: { equals: user.id } }, - data: { - email: json.user.email, - emailVerified: false, - emailVerificationToken: 'secret123', - }, - }); - } - return HttpResponse.json({ ok: true }); }); diff --git a/packages/crates-io-msw/handlers/users/update.test.js b/packages/crates-io-msw/handlers/users/update.test.js index 0456a47d530..52981929949 100644 --- a/packages/crates-io-msw/handlers/users/update.test.js +++ b/packages/crates-io-msw/handlers/users/update.test.js @@ -2,21 +2,6 @@ import { assert, test } from 'vitest'; import { db } from '../../index.js'; -test('updates the user with a new email address', async function () { - let user = db.user.create({ email: 'old@email.com' }); - db.mswSession.create({ user }); - - let body = JSON.stringify({ user: { email: 'new@email.com' } }); - let response = await fetch(`/api/v1/users/${user.id}`, { method: 'PUT', body }); - assert.strictEqual(response.status, 200); - assert.deepEqual(await response.json(), { ok: true }); - - user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.email, 'new@email.com'); - assert.strictEqual(user.emailVerified, false); - assert.strictEqual(user.emailVerificationToken, 'secret123'); -}); - test('updates the `publish_notifications` settings', async function () { let user = db.user.create(); db.mswSession.create({ user }); @@ -32,52 +17,38 @@ test('updates the `publish_notifications` settings', async function () { }); test('returns 403 when not logged in', async function () { - let user = db.user.create({ email: 'old@email.com' }); + let user = db.user.create({ emails: [db.email.create()] }); + assert.strictEqual(user.publishNotifications, true); - let body = JSON.stringify({ user: { email: 'new@email.com' } }); + let body = JSON.stringify({ user: { publish_notifications: false } }); let response = await fetch(`/api/v1/users/${user.id}`, { method: 'PUT', body }); assert.strictEqual(response.status, 403); assert.deepEqual(await response.json(), { errors: [{ detail: 'must be logged in to perform that action' }] }); user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.email, 'old@email.com'); + assert.strictEqual(user.publishNotifications, true); }); test('returns 400 when requesting the wrong user id', async function () { - let user = db.user.create({ email: 'old@email.com' }); + let user = db.user.create({ emails: [db.email.create()] }); + assert.strictEqual(user.publishNotifications, true); db.mswSession.create({ user }); - let body = JSON.stringify({ user: { email: 'new@email.com' } }); + let body = JSON.stringify({ user: { publish_notifications: false } }); let response = await fetch(`/api/v1/users/wrong-id`, { method: 'PUT', body }); assert.strictEqual(response.status, 400); assert.deepEqual(await response.json(), { errors: [{ detail: 'current user does not match requested user' }] }); user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.email, 'old@email.com'); + assert.strictEqual(user.publishNotifications, true); }); test('returns 400 when sending an invalid payload', async function () { - let user = db.user.create({ email: 'old@email.com' }); + let user = db.user.create({ emails: [db.email.create()] }); db.mswSession.create({ user }); let body = JSON.stringify({}); let response = await fetch(`/api/v1/users/${user.id}`, { method: 'PUT', body }); assert.strictEqual(response.status, 400); assert.deepEqual(await response.json(), { errors: [{ detail: 'invalid json request' }] }); - - user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.email, 'old@email.com'); -}); - -test('returns 400 when sending an empty email address', async function () { - let user = db.user.create({ email: 'old@email.com' }); - db.mswSession.create({ user }); - - let body = JSON.stringify({ user: { email: '' } }); - let response = await fetch(`/api/v1/users/${user.id}`, { method: 'PUT', body }); - assert.strictEqual(response.status, 400); - assert.deepEqual(await response.json(), { errors: [{ detail: 'empty email rejected' }] }); - - user = db.user.findFirst({ where: { id: user.id } }); - assert.strictEqual(user.email, 'old@email.com'); }); diff --git a/packages/crates-io-msw/index.js b/packages/crates-io-msw/index.js index 5973d4bb81a..e262a10e108 100644 --- a/packages/crates-io-msw/index.js +++ b/packages/crates-io-msw/index.js @@ -18,6 +18,7 @@ import crateOwnerInvitation from './models/crate-owner-invitation.js'; import crateOwnership from './models/crate-ownership.js'; import crate from './models/crate.js'; import dependency from './models/dependency.js'; +import email from './models/email.js'; import keyword from './models/keyword.js'; import mswSession from './models/msw-session.js'; import team from './models/team.js'; @@ -51,6 +52,7 @@ export const db = factory({ crateOwnership, crate, dependency, + email, keyword, mswSession, team, diff --git a/packages/crates-io-msw/models/api-token.test.js b/packages/crates-io-msw/models/api-token.test.js index 135b78352b9..d33546e1543 100644 --- a/packages/crates-io-msw/models/api-token.test.js +++ b/packages/crates-io-msw/models/api-token.test.js @@ -24,9 +24,7 @@ test('happy path', ({ expect }) => { "token": "6270739405881613", "user": { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "user-1@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 1, "isAdmin": false, diff --git a/packages/crates-io-msw/models/crate-owner-invitation.test.js b/packages/crates-io-msw/models/crate-owner-invitation.test.js index 32c10b97d52..29079addef0 100644 --- a/packages/crates-io-msw/models/crate-owner-invitation.test.js +++ b/packages/crates-io-msw/models/crate-owner-invitation.test.js @@ -56,9 +56,7 @@ test('happy path', ({ expect }) => { "id": 1, "invitee": { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "user-2@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 2, "isAdmin": false, @@ -71,9 +69,7 @@ test('happy path', ({ expect }) => { }, "inviter": { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "user-1@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 1, "isAdmin": false, diff --git a/packages/crates-io-msw/models/crate-ownership.test.js b/packages/crates-io-msw/models/crate-ownership.test.js index 00f6b389824..b40d8fec504 100644 --- a/packages/crates-io-msw/models/crate-ownership.test.js +++ b/packages/crates-io-msw/models/crate-ownership.test.js @@ -97,9 +97,7 @@ test('can set `user`', ({ expect }) => { "team": null, "user": { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "user-1@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 1, "isAdmin": false, diff --git a/packages/crates-io-msw/models/email.js b/packages/crates-io-msw/models/email.js new file mode 100644 index 00000000000..07933fa45bf --- /dev/null +++ b/packages/crates-io-msw/models/email.js @@ -0,0 +1,22 @@ +import { nullable, primaryKey } from '@mswjs/data'; + +import { applyDefault } from '../utils/defaults.js'; + +export default { + id: primaryKey(Number), + + email: String, + verified: Boolean, + verification_email_sent: Boolean, + primary: Boolean, + token: nullable(String), + + preCreate(attrs, counter) { + applyDefault(attrs, 'id', () => counter); + applyDefault(attrs, 'email', () => `foo@crates.io`); + applyDefault(attrs, 'verified', () => false); + applyDefault(attrs, 'verification_email_sent', () => false); + applyDefault(attrs, 'primary', () => false); + applyDefault(attrs, 'token', () => null); + }, +}; diff --git a/packages/crates-io-msw/models/email.test.js b/packages/crates-io-msw/models/email.test.js new file mode 100644 index 00000000000..0e23d7667d9 --- /dev/null +++ b/packages/crates-io-msw/models/email.test.js @@ -0,0 +1,19 @@ +import { test } from 'vitest'; + +import { db } from '../index.js'; + +test('default are applied', ({ expect }) => { + let email = db.email.create(); + expect(email).toMatchInlineSnapshot(` + { + "email": "foo@crates.io", + "id": 1, + "primary": false, + "token": null, + "verification_email_sent": false, + "verified": false, + Symbol(type): "email", + Symbol(primaryKey): "id", + } + `); +}); diff --git a/packages/crates-io-msw/models/msw-session.test.js b/packages/crates-io-msw/models/msw-session.test.js index 5a6874566c9..50d696e69ce 100644 --- a/packages/crates-io-msw/models/msw-session.test.js +++ b/packages/crates-io-msw/models/msw-session.test.js @@ -14,9 +14,7 @@ test('happy path', ({ expect }) => { "id": 1, "user": { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "user-1@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 1, "isAdmin": false, diff --git a/packages/crates-io-msw/models/user.js b/packages/crates-io-msw/models/user.js index 2d3ce6f22f4..ef68b87b651 100644 --- a/packages/crates-io-msw/models/user.js +++ b/packages/crates-io-msw/models/user.js @@ -10,9 +10,7 @@ export default { login: String, url: String, avatar: String, - email: nullable(String), - emailVerificationToken: nullable(String), - emailVerified: Boolean, + emails: manyOf('email'), isAdmin: Boolean, publishNotifications: Boolean, @@ -22,11 +20,8 @@ export default { applyDefault(attrs, 'id', () => counter); applyDefault(attrs, 'name', () => `User ${attrs.id}`); applyDefault(attrs, 'login', () => (attrs.name ? dasherize(attrs.name) : `user-${attrs.id}`)); - applyDefault(attrs, 'email', () => `${attrs.login}@crates.io`); applyDefault(attrs, 'url', () => `https://github.com/${attrs.login}`); applyDefault(attrs, 'avatar', () => 'https://avatars1.githubusercontent.com/u/14631425?v=4'); - applyDefault(attrs, 'emailVerificationToken', () => null); - applyDefault(attrs, 'emailVerified', () => Boolean(attrs.email && !attrs.emailVerificationToken)); applyDefault(attrs, 'isAdmin', () => false); applyDefault(attrs, 'publishNotifications', () => true); }, diff --git a/packages/crates-io-msw/models/user.test.js b/packages/crates-io-msw/models/user.test.js index e3db559e569..7870c92444c 100644 --- a/packages/crates-io-msw/models/user.test.js +++ b/packages/crates-io-msw/models/user.test.js @@ -7,9 +7,7 @@ test('default are applied', ({ expect }) => { expect(user).toMatchInlineSnapshot(` { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "user-1@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 1, "isAdmin": false, @@ -28,9 +26,7 @@ test('name can be set', ({ expect }) => { expect(user).toMatchInlineSnapshot(` { "avatar": "https://avatars1.githubusercontent.com/u/14631425?v=4", - "email": "john-doe@crates.io", - "emailVerificationToken": null, - "emailVerified": true, + "emails": [], "followedCrates": [], "id": 1, "isAdmin": false, diff --git a/packages/crates-io-msw/serializers/email.js b/packages/crates-io-msw/serializers/email.js new file mode 100644 index 00000000000..68bd8638c40 --- /dev/null +++ b/packages/crates-io-msw/serializers/email.js @@ -0,0 +1,9 @@ +import { serializeModel } from '../utils/serializers.js'; + +export function serializeEmail(email) { + let serialized = serializeModel(email); + + delete serialized.token; + + return serialized; +} diff --git a/packages/crates-io-msw/serializers/user.js b/packages/crates-io-msw/serializers/user.js index 9f3725977af..c902ef69c7d 100644 --- a/packages/crates-io-msw/serializers/user.js +++ b/packages/crates-io-msw/serializers/user.js @@ -1,18 +1,16 @@ import { serializeModel } from '../utils/serializers.js'; +import { serializeEmail } from './email.js'; export function serializeUser(user, { removePrivateData = true } = {}) { let serialized = serializeModel(user); + serialized.emails = user.emails.map(email => serializeEmail(email)); if (removePrivateData) { - delete serialized.email; - delete serialized.email_verified; + delete serialized.emails; delete serialized.is_admin; delete serialized.publish_notifications; - } else { - serialized.email_verification_sent = serialized.email_verified || Boolean(serialized.email_verification_token); } - delete serialized.email_verification_token; delete serialized.followed_crates; return serialized; 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..50c17572cdb 100644 --- a/src/controllers/session.rs +++ b/src/controllers/session.rs @@ -2,14 +2,14 @@ use crate::app::AppState; use crate::email::EmailMessage; use crate::email::Emails; use crate::middleware::log_request::RequestLogExt; -use crate::models::{NewEmail, NewUser, User}; +use crate::models::{Email, NewEmail, NewUser, User}; use crate::schema::users; use crate::util::diesel::is_read_only_error; use crate::util::errors::{AppResult, bad_request, server_error}; use crate::views::EncodableMe; use axum::Json; use axum::extract::{FromRequestParts, Query}; -use crates_io_github::GitHubUser; +use crates_io_github::{GitHubEmail, GitHubUser}; use crates_io_session::SessionExtension; use diesel::prelude::*; use diesel_async::scoped_futures::ScopedFutureExt; @@ -49,6 +49,7 @@ pub async fn begin_session(app: AppState, session: SessionExtension) -> Json emails, + Err(err) => { + warn!("Failed to fetch user emails from GitHub: {err}"); + // Continue anyway, user may have denied the user:email scope on purpose + // but we could have their public email from the user info. + if let Some(gh_email) = &ghuser.email { + vec![GitHubEmail { + email: gh_email.to_string(), + primary: true, + verified: false, + }] + } else { + vec![] + } + } + }; let mut conn = app.db_write().await?; - let user = save_user_to_database(&ghuser, token.secret(), &app.emails, &mut conn).await?; + let user = save_user_to_database( + &ghuser, + &mut ghemails, + token.secret(), + &app.emails, + &mut conn, + ) + .await?; // Log in by setting a cookie and the middleware authentication session.insert("user_id".to_string(), user.id.to_string()); @@ -128,6 +160,7 @@ pub async fn authorize_session( pub async fn save_user_to_database( user: &GitHubUser, + user_emails: &mut [GitHubEmail], access_token: &str, emails: &Emails, conn: &mut AsyncPgConnection, @@ -140,7 +173,7 @@ pub async fn save_user_to_database( .gh_access_token(access_token) .build(); - match create_or_update_user(&new_user, user.email.as_deref(), emails, conn).await { + match create_or_update_user(&new_user, user_emails, emails, conn).await { Ok(user) => Ok(user), Err(error) if is_read_only_error(&error) => { // If we're in read only mode, we can't update their details @@ -157,7 +190,7 @@ pub async fn save_user_to_database( /// and sends a confirmation email to the user. async fn create_or_update_user( new_user: &NewUser<'_>, - email: Option<&str>, + user_emails: &mut [GitHubEmail], emails: &Emails, conn: &mut AsyncPgConnection, ) -> QueryResult { @@ -165,27 +198,49 @@ async fn create_or_update_user( async move { let user = new_user.insert_or_update(conn).await?; + // Count the number of existing emails to determine if we need to + // mark the first email address as primary. + let mut email_count: i64 = Email::belonging_to(&user).count().get_result(conn).await?; + + // Sort the GitHub emails by primary status so that the primary email is inserted + // first, and therefore will be marked as primary in our database. + user_emails.sort_by(|a, b| { + if a.primary && !b.primary { + std::cmp::Ordering::Less + } else if !a.primary && b.primary { + std::cmp::Ordering::Greater + } else { + std::cmp::Ordering::Equal + } + }); + // To send the user an account verification email - if let Some(user_email) = email { + for user_email in user_emails { + email_count += 1; // Increment the count so that we don't mark subsequent emails as primary + let new_email = NewEmail::builder() .user_id(user.id) - .email(user_email) + .email(&user_email.email) + .verified(user_email.verified) // we can trust GitHub's verification + .primary(email_count == 1) // Mark as primary if this is the user's first email .build(); - if let Some(token) = new_email.insert_if_missing(conn).await? { + if let Some(saved_email) = new_email.insert_if_missing(conn).await? + && !new_email.verified + { 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() }, ); match email { Ok(email) => { // Swallows any error. Some users might insert an invalid email address here. - let _ = emails.send(user_email, email).await; + let _ = emails.send(&saved_email.email, email).await; } Err(error) => { warn!("Failed to render user confirmation email template: {error}"); @@ -242,7 +297,19 @@ mod tests { id: -1, avatar_url: None, }; - let result = save_user_to_database(&gh_user, "arbitrary_token", &emails, &mut conn).await; + let mut gh_emails = vec![GitHubEmail { + email: gh_user.email.clone().unwrap(), + primary: true, + verified: false, + }]; + let result = save_user_to_database( + &gh_user, + &mut gh_emails, + "arbitrary_token", + &emails, + &mut conn, + ) + .await; assert!( result.is_ok(), diff --git a/src/controllers/user.rs b/src/controllers/user.rs index 4a604d16077..cb5985a3b7d 100644 --- a/src/controllers/user.rs +++ b/src/controllers/user.rs @@ -1,8 +1,11 @@ pub mod email_notifications; pub mod email_verification; +pub mod emails; pub mod me; pub mod other; pub mod update; -pub use email_verification::resend_email_verification; +#[allow(deprecated)] +pub use email_verification::{resend_email_verification, resend_email_verification_all}; +pub use emails::{create_email, delete_email}; pub use update::update_user; diff --git a/src/controllers/user/email_verification.rs b/src/controllers/user/email_verification.rs index 29479db6c84..6b296fb33ff 100644 --- a/src/controllers/user/email_verification.rs +++ b/src/controllers/user/email_verification.rs @@ -5,15 +5,26 @@ use crate::email::EmailMessage; use crate::models::Email; use crate::util::errors::AppResult; use crate::util::errors::{BoxedAppError, bad_request}; +use crate::views::EncodableEmail; +use axum::Json; use axum::extract::Path; use crates_io_database::schema::emails; use diesel::dsl::sql; use diesel::prelude::*; +use diesel::result::OptionalExtension; use diesel_async::scoped_futures::ScopedFutureExt; use diesel_async::{AsyncConnection, RunQueryDsl}; use http::request::Parts; use minijinja::context; use secrecy::ExposeSecret; +use serde::Serialize; + +#[derive(Serialize, utoipa::ToSchema)] +pub struct EmailConfirmResponse { + #[schema(example = true)] + ok: bool, + email: EncodableEmail, +} /// Marks the email belonging to the given token as verified. #[utoipa::path( @@ -23,32 +34,38 @@ use secrecy::ExposeSecret; ("email_token" = String, Path, description = "Secret verification token sent to the user's email address"), ), tag = "users", - responses((status = 200, description = "Successful Response", body = inline(OkResponse))), + responses((status = 200, description = "Successful Response", body = inline(EmailConfirmResponse))), )] pub async fn confirm_user_email( state: AppState, Path(token): Path, -) -> AppResult { +) -> AppResult> { let mut conn = state.db_write().await?; - let updated_rows = diesel::update(emails::table.filter(emails::token.eq(&token))) + let confirmed_email = diesel::update(emails::table.filter(emails::token.eq(&token))) .set(emails::verified.eq(true)) - .execute(&mut conn) - .await?; - - if updated_rows == 0 { - return Err(bad_request("Email belonging to token not found.")); + .returning(Email::as_returning()) + .get_result(&mut conn) + .await + .optional()?; + + if let Some(confirmed_email) = confirmed_email { + Ok(Json(EmailConfirmResponse { + ok: true, + email: confirmed_email.into(), + })) + } else { + Err(bad_request("Email belonging to token not found.")) } - - Ok(OkResponse::new()) } -/// Regenerate and send an email verification token. +/// Regenerate and send an email verification token for the given email. #[utoipa::path( put, - path = "/api/v1/users/{id}/resend", + path = "/api/v1/users/{user_id}/emails/{id}/resend", params( - ("id" = i32, Path, description = "ID of the user"), + ("user_id" = i32, Path, description = "ID of the user"), + ("id" = i32, Path, description = "ID of the email"), ), security( ("api_token" = []), @@ -59,7 +76,7 @@ pub async fn confirm_user_email( )] pub async fn resend_email_verification( state: AppState, - Path(param_user_id): Path, + Path((param_user_id, email_id)): Path<(i32, i32)>, req: Parts, ) -> AppResult { let mut conn = state.db_write().await?; @@ -70,16 +87,23 @@ pub async fn resend_email_verification( return Err(bad_request("current user does not match requested user")); } + // Generate a new token for the email, if it exists and is unverified conn.transaction(|conn| { async move { - let email: Email = diesel::update(Email::belonging_to(auth.user())) - .set(emails::token.eq(sql("DEFAULT"))) - .returning(Email::as_returning()) - .get_result(conn) - .await - .optional()? - .ok_or_else(|| bad_request("Email could not be found"))?; - + let email: Email = diesel::update( + emails::table + .filter(emails::id.eq(email_id)) + .filter(emails::user_id.eq(auth.user_id())) + .filter(emails::verified.eq(false)), + ) + .set(emails::token.eq(sql("DEFAULT"))) + .returning(Email::as_returning()) + .get_result(conn) + .await + .optional()? + .ok_or_else(|| bad_request("Email not found or already verified"))?; + + // Send the updated token via email let email_message = EmailMessage::from_template( "user_confirm", context! { @@ -94,7 +118,81 @@ pub async fn resend_email_verification( .emails .send(&email.email, email_message) .await - .map_err(BoxedAppError::from) + .map_err(BoxedAppError::from)?; + + Ok::<(), BoxedAppError>(()) + } + .scope_boxed() + }) + .await?; + + Ok(OkResponse::new()) +} + +/// Regenerate and send an email verification token for any unverified email of the current user. +/// Deprecated endpoint, use `PUT /api/v1/user/{user_id}/emails/{id}/resend` instead. +#[utoipa::path( + put, + path = "/api/v1/users/{id}/resend", + params( + ("id" = i32, Path, description = "ID of the user"), + ), + security( + ("api_token" = []), + ("cookie" = []), + ), + tag = "users", + responses((status = 200, description = "Successful Response", body = inline(OkResponse))), +)] +#[deprecated] +pub async fn resend_email_verification_all( + state: AppState, + Path(param_user_id): Path, + req: Parts, +) -> AppResult { + let mut conn = state.db_write().await?; + let auth = AuthCheck::default().check(&req, &mut conn).await?; + + // need to check if current user matches user to be updated + if auth.user_id() != param_user_id { + return Err(bad_request("current user does not match requested user")); + } + + conn.transaction(|conn| { + async move { + let emails: Vec = diesel::update( + emails::table + .filter(emails::user_id.eq(auth.user_id())) + .filter(emails::verified.eq(false)), + ) + .set(emails::token.eq(sql("DEFAULT"))) + .returning(Email::as_returning()) + .get_results(conn) + .await?; + + if emails.is_empty() { + return Err(bad_request("No unverified emails found")); + } + + for email in emails { + let email_message = EmailMessage::from_template( + "user_confirm", + context! { + user_name => auth.user().gh_login, + domain => state.emails.domain, + token => email.token.expose_secret() + }, + ) + .map_err(|_| bad_request("Failed to render email template"))?; + + state + .emails + .send(&email.email, email_message) + .await + .map_err(BoxedAppError::from)?; + } + + Ok(()) } .scope_boxed() }) @@ -109,7 +207,7 @@ mod tests { use insta::assert_snapshot; #[tokio::test(flavor = "multi_thread")] - async fn test_no_auth() { + async fn test_legacy_no_auth() { let (app, anon, user) = TestApp::init().with_user().await; let url = format!("/api/v1/users/{}/resend", user.as_model().id); @@ -121,7 +219,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] - async fn test_wrong_user() { + async fn test_legacy_wrong_user() { let (app, _anon, user) = TestApp::init().with_user().await; let user2 = app.db_new_user("bar").await; @@ -134,9 +232,13 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] - async fn test_happy_path() { + async fn test_legacy_happy_path() { let (app, _anon, user) = TestApp::init().with_user().await; + // Create a new email to be verified, inserting directly into the database so that verification is not sent + let _new_email = user.db_new_email("bar@example.com", false, false).await; + + // Request a verification email let url = format!("/api/v1/users/{}/resend", user.as_model().id); let response = user.put::<()>(&url, "").await; assert_snapshot!(response.status(), @"200 OK"); diff --git a/src/controllers/user/emails.rs b/src/controllers/user/emails.rs new file mode 100644 index 00000000000..546a2e07a8c --- /dev/null +++ b/src/controllers/user/emails.rs @@ -0,0 +1,198 @@ +use crate::app::AppState; +use crate::auth::AuthCheck; +use crate::controllers::helpers::OkResponse; +use crate::email::EmailMessage; +use crate::models::{Email, NewEmail}; +use crate::util::errors::{AppResult, bad_request, not_found, server_error}; +use crate::views::EncodableEmail; +use axum::Json; +use axum::extract::{FromRequest, Path}; +use crates_io_database::schema::emails; +use diesel::prelude::*; +use diesel_async::RunQueryDsl; +use http::request::Parts; +use lettre::Address; +use minijinja::context; +use secrecy::ExposeSecret; +use serde::Deserialize; + +#[derive(Deserialize, FromRequest, utoipa::ToSchema)] +#[from_request(via(Json))] +pub struct EmailCreate { + email: String, +} + +/// Add a new email address to a user profile. +#[utoipa::path( + post, + path = "/api/v1/users/{id}/emails", + params( + ("id" = i32, Path, description = "ID of the user"), + ), + request_body = inline(EmailCreate), + security( + ("api_token" = []), + ("cookie" = []), + ), + tag = "users", + responses((status = 200, description = "Successful Response", body = EncodableEmail)), +)] +pub async fn create_email( + state: AppState, + Path(param_user_id): Path, + req: Parts, + email: EmailCreate, +) -> AppResult> { + let mut conn = state.db_write().await?; + let auth = AuthCheck::default().check(&req, &mut conn).await?; + + // need to check if current user matches user to be updated + if auth.user_id() != param_user_id { + return Err(bad_request("current user does not match requested user")); + } + + let user_email = email.email.trim(); + + if user_email.is_empty() { + return Err(bad_request("empty email rejected")); + } + + user_email + .parse::
() + .map_err(|_| bad_request("invalid email address"))?; + + // fetch count of user's current emails to determine if we need to mark the new email as primary + let email_count: i64 = Email::belonging_to(&auth.user()) + .count() + .get_result(&mut conn) + .await + .map_err(|_| server_error("Error fetching existing emails"))?; + + let saved_email = NewEmail::builder() + .user_id(auth.user().id) + .email(user_email) + .primary(email_count == 0) // Mark as primary if this is the first email + .build() + .insert_if_missing(&mut conn) + .await + .map_err(|e| server_error(format!("{e}")))?; + + let saved_email = match saved_email { + Some(email) => email, + None => return Err(bad_request("email already exists")), + }; + + let verification_message = EmailMessage::from_template( + "user_confirm", + context! { + user_name => auth.user().gh_login, + domain => state.emails.domain, + token => saved_email.token.expose_secret() + }, + ) + .map_err(|_| server_error("Failed to render email template"))?; + + state + .emails + .send(&saved_email.email, verification_message) + .await?; + + Ok(Json(EncodableEmail::from(saved_email))) +} + +/// Delete an email address from a user profile. +#[utoipa::path( + delete, + path = "/api/v1/users/{id}/emails/{email_id}", + params( + ("id" = i32, Path, description = "ID of the user"), + ("email_id" = i32, Path, description = "ID of the email to delete"), + ), + security( + ("api_token" = []), + ("cookie" = []), + ), + tag = "users", + responses((status = 200, description = "Successful Response", body = inline(OkResponse))), +)] +pub async fn delete_email( + state: AppState, + Path((param_user_id, email_id)): Path<(i32, i32)>, + req: Parts, +) -> AppResult { + let mut conn = state.db_write().await?; + let auth = AuthCheck::default().check(&req, &mut conn).await?; + + // need to check if current user matches user to be updated + if auth.user_id() != param_user_id { + return Err(bad_request("current user does not match requested user")); + } + + let email = Email::belonging_to(&auth.user()) + .filter(emails::id.eq(email_id)) + .select(Email::as_select()) + .get_result(&mut conn) + .await + .map_err(|_| not_found())?; + + if email.primary { + return Err(bad_request( + "cannot delete primary email, please set another email as primary first", + )); + } + + diesel::delete(&email) + .execute(&mut conn) + .await + .map_err(|_| server_error("Error in deleting email"))?; + + Ok(OkResponse::new()) +} + +/// Mark a specific email address as the primary email. This will cause notifications to be sent to this email address. +#[utoipa::path( + put, + path = "/api/v1/users/{id}/emails/{email_id}/set_primary", + params( + ("id" = i32, Path, description = "ID of the user"), + ("email_id" = i32, Path, description = "ID of the email to set as primary"), + ), + security( + ("api_token" = []), + ("cookie" = []), + ), + tag = "users", + responses((status = 200, description = "Successful Response", body = inline(OkResponse))), +)] +pub async fn set_primary_email( + state: AppState, + Path((param_user_id, email_id)): Path<(i32, i32)>, + req: Parts, +) -> AppResult { + let mut conn = state.db_write().await?; + let auth = AuthCheck::default().check(&req, &mut conn).await?; + + // need to check if current user matches user to be updated + if auth.user_id() != param_user_id { + return Err(bad_request("current user does not match requested user")); + } + + let email = Email::belonging_to(&auth.user()) + .filter(emails::id.eq(email_id)) + .select(Email::as_select()) + .get_result(&mut conn) + .await + .map_err(|_| not_found())?; + + if email.primary { + return Err(bad_request("email is already primary")); + } + + diesel::sql_query("SELECT promote_email_to_primary($1)") + .bind::(email_id) + .execute(&mut conn) + .await + .map_err(|_| server_error("Error in marking email as primary"))?; + + Ok(OkResponse::new()) +} diff --git a/src/controllers/user/me.rs b/src/controllers/user/me.rs index 046ebcc86d3..c54076f9b91 100644 --- a/src/controllers/user/me.rs +++ b/src/controllers/user/me.rs @@ -4,10 +4,12 @@ use crate::controllers::helpers::Paginate; use crate::controllers::helpers::pagination::{Paginated, PaginationOptions}; use crate::models::krate::CrateName; use crate::models::{CrateOwner, Follow, OwnerKind, User, Version, VersionOwnerAction}; -use crate::schema::{crate_owners, crates, emails, follows, users, versions}; +use crate::schema::{crate_owners, crates, follows, users, versions}; use crate::util::errors::AppResult; use crate::views::{EncodableMe, EncodablePrivateUser, EncodableVersion, OwnedCrate}; use axum::Json; +use crates_io_database::models::Email; +use crates_io_database::schema::emails; use diesel::prelude::*; use diesel_async::RunQueryDsl; use futures_util::FutureExt; @@ -24,31 +26,24 @@ use serde::Serialize; )] pub async fn get_authenticated_user(app: AppState, req: Parts) -> AppResult> { let mut conn = app.db_read_prefer_primary().await?; - let user_id = AuthCheck::only_cookie() - .check(&req, &mut conn) - .await? - .user_id(); - - let ((user, verified, email, verification_sent), owned_crates) = tokio::try_join!( - users::table - .find(user_id) - .left_join(emails::table) - .select(( - User::as_select(), - emails::verified.nullable(), - emails::email.nullable(), - emails::token_generated_at.nullable().is_not_null(), - )) - .first::<(User, Option, Option, bool)>(&mut conn) - .boxed(), - CrateOwner::by_owner_kind(OwnerKind::User) - .inner_join(crates::table) - .filter(crate_owners::owner_id.eq(user_id)) - .select((crates::id, crates::name, crate_owners::email_notifications)) - .order(crates::name.asc()) - .load(&mut conn) - .boxed() - )?; + let user = AuthCheck::only_cookie().check(&req, &mut conn).await?; + + let emails_query = Email::belonging_to(user.user()) + .select(Email::as_select()) + .order(emails::id.asc()) + .load(&mut conn) + .boxed(); + + let owned_crates_query = CrateOwner::by_owner_kind(OwnerKind::User) + .inner_join(crates::table) + .filter(crate_owners::owner_id.eq(user.user_id())) + .select((crates::id, crates::name, crate_owners::email_notifications)) + .order(crates::name.asc()) + .load(&mut conn) + .boxed(); + + let (emails, owned_crates): (Vec, Vec<(i32, String, bool)>) = + tokio::try_join!(emails_query, owned_crates_query)?; let owned_crates = owned_crates .into_iter() @@ -59,10 +54,8 @@ pub async fn get_authenticated_user(app: AppState, req: Parts) -> AppResult Subject: crates.io: Please confirm your email address Content-Type: text/plain; charset=utf-8 diff --git a/src/controllers/user/update.rs b/src/controllers/user/update.rs index 7d5174974b1..51352a86c31 100644 --- a/src/controllers/user/update.rs +++ b/src/controllers/user/update.rs @@ -2,7 +2,7 @@ use crate::app::AppState; use crate::auth::AuthCheck; use crate::controllers::helpers::OkResponse; use crate::email::EmailMessage; -use crate::models::NewEmail; +use crate::models::{Email, NewEmail}; use crate::schema::users; use crate::util::errors::{AppResult, bad_request, server_error}; use axum::Json; @@ -16,20 +16,27 @@ use secrecy::ExposeSecret; use serde::Deserialize; use tracing::warn; -#[derive(Deserialize)] +#[derive(Deserialize, utoipa::ToSchema)] pub struct UserUpdate { user: User, } -#[derive(Deserialize)] +#[derive(Deserialize, utoipa::ToSchema)] +#[schema(as = UserUpdateParameters)] pub struct User { + #[deprecated(note = "Use `/api/v1/users/{id}/emails` instead.")] email: Option, publish_notifications: Option, } /// Update user settings. /// -/// This endpoint allows users to update their email address and publish notifications settings. +/// This endpoint allows users to manage publish notifications settings. +/// +/// You may provide an `email` parameter to add a new email address to the user's profile, but +/// this is for legacy support only and will be removed in the future. +/// +/// For managing email addresses, please use the `/api/v1/users/{id}/emails` endpoints instead. /// /// The `id` parameter needs to match the ID of the currently authenticated user. #[utoipa::path( @@ -38,6 +45,7 @@ pub struct User { params( ("user" = i32, Path, description = "ID of the user"), ), + request_body = inline(UserUpdate), security( ("api_token" = []), ("cookie" = []), @@ -95,6 +103,7 @@ pub async fn update_user( } } + #[allow(deprecated)] if let Some(user_email) = &user_update.user.email { let user_email = user_email.trim(); @@ -106,35 +115,45 @@ pub async fn update_user( .parse::
() .map_err(|_| bad_request("invalid email address"))?; - let new_email = NewEmail::builder() + // Check if this is the first email for the user, because if so, we need to mark it as the primary + let existing_email_count: i64 = Email::belonging_to(&user) + .count() + .get_result(&mut conn) + .await + .map_err(|_| server_error("Error fetching existing emails"))?; + + let saved_email = NewEmail::builder() .user_id(user.id) .email(user_email) - .build(); - - let token = new_email.insert_or_update(&mut conn).await; - let token = token.map_err(|_| server_error("Error in creating token"))?; - - // 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 - // we're trying to silently use their invalid address during signup and can't send them an - // email. They'll then have to provide a valid email address. - let email = EmailMessage::from_template( - "user_confirm", - context! { - user_name => user.gh_login, - domain => state.emails.domain, - token => token.expose_secret() - }, - ); - - match email { - Ok(email) => { - let _ = state.emails.send(user_email, email).await; - } - Err(error) => { - warn!("Failed to render user confirmation email template: {error}"); + .primary(existing_email_count < 1) // Mark as primary if this is the first email + .build() + .insert_if_missing(&mut conn) + .await + .map_err(|_| server_error("Error saving email"))?; + + if let Some(saved_email) = saved_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 + // we're trying to silently use their invalid address during signup and can't send them an + // email. They'll then have to provide a valid email address. + let email = EmailMessage::from_template( + "user_confirm", + context! { + user_name => user.gh_login, + domain => state.emails.domain, + token => saved_email.token.expose_secret() + }, + ); + + match email { + Ok(email) => { + let _ = state.emails.send(user_email, email).await; + } + Err(error) => { + warn!("Failed to render user confirmation email template: {error}"); + } } - }; + } } Ok(OkResponse::new()) diff --git a/src/router.rs b/src/router.rs index 91d598f0cb5..991b5de94ff 100644 --- a/src/router.rs +++ b/src/router.rs @@ -81,8 +81,14 @@ pub fn build_axum_router(state: AppState) -> Router<()> { user::email_notifications::update_email_notifications )) .routes(routes!(summary::get_summary)) + .routes(routes!(user::emails::create_email)) + .routes(routes!(user::emails::delete_email)) + .routes(routes!(user::emails::set_primary_email)) .routes(routes!(user::email_verification::confirm_user_email)) .routes(routes!(user::email_verification::resend_email_verification)) + .routes(routes!( + user::email_verification::resend_email_verification_all + )) .routes(routes!(site_metadata::get_site_metadata)) // Session management .routes(routes!(session::begin_session)) 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..2b1f9ff13f5 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,8 @@ expression: response.json() ] }, "email": { - "description": "The user's email address, if set.", + "deprecated": true, + "description": "The user's primary email address, if set.", "example": "kate@morgan.dev", "type": [ "string", @@ -96,15 +97,33 @@ expression: response.json() ] }, "email_verification_sent": { - "description": "Whether the user's email address verification email has been sent.", + "deprecated": true, + "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.", + "deprecated": true, + "description": "Whether the user's primary email address, if set, has been verified.", "example": true, "type": "boolean" }, + "emails": { + "description": "The user's email addresses.", + "example": [ + { + "email": "user@example.com", + "id": 42, + "primary": true, + "verification_email_sent": true, + "verified": true + } + ], + "items": { + "$ref": "#/components/schemas/Email" + }, + "type": "array" + }, "id": { "description": "An opaque identifier for the user.", "example": 42, @@ -146,6 +165,7 @@ expression: response.json() "required": [ "id", "login", + "emails", "email_verified", "email_verification_sent", "is_admin", @@ -497,6 +517,44 @@ expression: response.json() ], "type": "object" }, + "Email": { + "properties": { + "email": { + "description": "The email address.", + "example": "user@example.com", + "type": "string" + }, + "id": { + "description": "An opaque identifier for the email.", + "example": 42, + "format": "int32", + "type": "integer" + }, + "primary": { + "description": "Whether this is the user's primary email address, meaning notifications will be sent here.", + "example": true, + "type": "boolean" + }, + "verification_email_sent": { + "description": "Whether the verification email has been sent.", + "example": true, + "type": "boolean" + }, + "verified": { + "description": "Whether the email address has been verified.", + "example": true, + "type": "boolean" + } + }, + "required": [ + "id", + "email", + "verified", + "verification_email_sent", + "primary" + ], + "type": "object" + }, "EncodableApiTokenWithToken": { "allOf": [ { @@ -963,6 +1021,24 @@ expression: response.json() ], "type": "object" }, + "UserUpdateParameters": { + "properties": { + "email": { + "deprecated": true, + "type": [ + "string", + "null" + ] + }, + "publish_notifications": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "Version": { "properties": { "audit_actions": { @@ -1694,13 +1770,17 @@ expression: response.json() "application/json": { "schema": { "properties": { + "email": { + "$ref": "#/components/schemas/Email" + }, "ok": { "example": true, "type": "boolean" } }, "required": [ - "ok" + "ok", + "email" ], "type": "object" } @@ -4381,9 +4461,189 @@ expression: response.json() ] } }, + "/api/v1/users/{id}/emails": { + "post": { + "operationId": "create_email", + "parameters": [ + { + "description": "ID of the user", + "in": "path", + "name": "id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "email": { + "type": "string" + } + }, + "required": [ + "email" + ], + "type": "object" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Email" + } + } + }, + "description": "Successful Response" + } + }, + "security": [ + { + "api_token": [] + }, + { + "cookie": [] + } + ], + "summary": "Add a new email address to a user profile.", + "tags": [ + "users" + ] + } + }, + "/api/v1/users/{id}/emails/{email_id}": { + "delete": { + "operationId": "delete_email", + "parameters": [ + { + "description": "ID of the user", + "in": "path", + "name": "id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + }, + { + "description": "ID of the email to delete", + "in": "path", + "name": "email_id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "ok": { + "example": true, + "type": "boolean" + } + }, + "required": [ + "ok" + ], + "type": "object" + } + } + }, + "description": "Successful Response" + } + }, + "security": [ + { + "api_token": [] + }, + { + "cookie": [] + } + ], + "summary": "Delete an email address from a user profile.", + "tags": [ + "users" + ] + } + }, + "/api/v1/users/{id}/emails/{email_id}/set_primary": { + "put": { + "operationId": "set_primary_email", + "parameters": [ + { + "description": "ID of the user", + "in": "path", + "name": "id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + }, + { + "description": "ID of the email to set as primary", + "in": "path", + "name": "email_id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "ok": { + "example": true, + "type": "boolean" + } + }, + "required": [ + "ok" + ], + "type": "object" + } + } + }, + "description": "Successful Response" + } + }, + "security": [ + { + "api_token": [] + }, + { + "cookie": [] + } + ], + "summary": "Mark a specific email address as the primary email. This will cause notifications to be sent to this email address.", + "tags": [ + "users" + ] + } + }, "/api/v1/users/{id}/resend": { "put": { - "operationId": "resend_email_verification", + "deprecated": true, + "operationId": "resend_email_verification_all", "parameters": [ { "description": "ID of the user", @@ -4425,7 +4685,7 @@ expression: response.json() "cookie": [] } ], - "summary": "Regenerate and send an email verification token.", + "summary": "Regenerate and send an email verification token for any unverified email of the current user.\nDeprecated endpoint, use `PUT /api/v1/user/{user_id}/emails/{id}/resend` instead.", "tags": [ "users" ] @@ -4477,6 +4737,66 @@ expression: response.json() ] } }, + "/api/v1/users/{user_id}/emails/{id}/resend": { + "put": { + "operationId": "resend_email_verification", + "parameters": [ + { + "description": "ID of the user", + "in": "path", + "name": "user_id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + }, + { + "description": "ID of the email", + "in": "path", + "name": "id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "ok": { + "example": true, + "type": "boolean" + } + }, + "required": [ + "ok" + ], + "type": "object" + } + } + }, + "description": "Successful Response" + } + }, + "security": [ + { + "api_token": [] + }, + { + "cookie": [] + } + ], + "summary": "Regenerate and send an email verification token for the given email.", + "tags": [ + "users" + ] + } + }, "/api/v1/users/{user}": { "get": { "operationId": "find_user", @@ -4517,7 +4837,7 @@ expression: response.json() ] }, "put": { - "description": "This endpoint allows users to update their email address and publish notifications settings.\n\nThe `id` parameter needs to match the ID of the currently authenticated user.", + "description": "This endpoint allows users to manage publish notifications settings.\n\nYou may provide an `email` parameter to add a new email address to the user's profile, but\nthis is for legacy support only and will be removed in the future.\n\nFor managing email addresses, please use the `/api/v1/users/{id}/emails` endpoints instead.\n\nThe `id` parameter needs to match the ID of the currently authenticated user.", "operationId": "update_user", "parameters": [ { @@ -4531,6 +4851,24 @@ expression: response.json() } } ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "user": { + "$ref": "#/components/schemas/UserUpdateParameters" + } + }, + "required": [ + "user" + ], + "type": "object" + } + } + }, + "required": true + }, "responses": { "200": { "content": { diff --git a/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-4.snap b/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-4.snap index 5564b16de5e..aaf6f7e42b2 100644 --- a/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-4.snap +++ b/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-4.snap @@ -9,6 +9,15 @@ expression: response.json() "email": "foo@example.com", "email_verification_sent": true, "email_verified": true, + "emails": [ + { + "email": "foo@example.com", + "id": 1, + "primary": true, + "verification_email_sent": true, + "verified": true + } + ], "id": 1, "is_admin": false, "login": "foo", diff --git a/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-6.snap b/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-6.snap index b0ffc3e7fc8..1d17ee10ea8 100644 --- a/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-6.snap +++ b/src/tests/routes/me/snapshots/crates_io__tests__routes__me__get__me-6.snap @@ -15,6 +15,15 @@ expression: response.json() "email": "foo@example.com", "email_verification_sent": true, "email_verified": true, + "emails": [ + { + "email": "foo@example.com", + "id": 1, + "primary": true, + "verification_email_sent": true, + "verified": true + } + ], "id": 1, "is_admin": false, "login": "foo", diff --git a/src/tests/routes/users/email_verification.rs b/src/tests/routes/users/email_verification.rs new file mode 100644 index 00000000000..07c4ca28557 --- /dev/null +++ b/src/tests/routes/users/email_verification.rs @@ -0,0 +1,54 @@ +use super::emails::MockEmailHelper; +use crate::tests::util::{RequestHelper, Response, TestApp}; +use insta::assert_snapshot; + +pub trait MockEmailVerificationHelper: RequestHelper { + async fn resend_confirmation(&self, user_id: i32, email_id: i32) -> Response<()> { + let url = format!("/api/v1/users/{user_id}/emails/{email_id}/resend"); + self.put(&url, &[] as &[u8]).await + } +} + +impl MockEmailVerificationHelper for crate::tests::util::MockCookieUser {} +impl MockEmailVerificationHelper for crate::tests::util::MockAnonymousUser {} + +#[tokio::test(flavor = "multi_thread")] +async fn test_no_auth() { + let (app, anon, user) = TestApp::init().with_user().await; + + let response = anon.resend_confirmation(user.as_model().id, 1).await; + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"this action requires authentication"}]}"#); + + assert_eq!(app.emails().await.len(), 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_wrong_user() { + let (app, _anon, user) = TestApp::init().with_user().await; + let user2 = app.db_new_user("bar").await; + let response = user.resend_confirmation(user2.as_model().id, 1).await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"current user does not match requested user"}]}"#); + assert_eq!(app.emails().await.len(), 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_happy_path() { + let (app, _anon, user) = TestApp::init().with_user().await; + + // Add an email to the user + let response = user.add_email(user.as_model().id, "user@example.com").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"id":2,"email":"user@example.com","verified":false,"verification_email_sent":true,"primary":false}"#); + + let response = user + .resend_confirmation( + user.as_model().id, + response.json()["id"].as_u64().unwrap() as i32, + ) + .await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"ok":true}"#); + assert_snapshot!(app.emails_snapshot().await); +} diff --git a/src/tests/routes/users/emails.rs b/src/tests/routes/users/emails.rs new file mode 100644 index 00000000000..1a172f187a1 --- /dev/null +++ b/src/tests/routes/users/emails.rs @@ -0,0 +1,220 @@ +use crate::tests::util::{RequestHelper, Response, TestApp}; +use insta::assert_snapshot; +use serde_json::json; + +pub trait MockEmailHelper: RequestHelper { + async fn add_email(&self, user_id: i32, email: &str) -> Response<()> { + let body = json!({"email": email}); + let url = format!("/api/v1/users/{user_id}/emails"); + self.post(&url, body.to_string()).await + } + + async fn delete_email(&self, user_id: i32, email_id: i32) -> Response<()> { + let url = format!("/api/v1/users/{user_id}/emails/{email_id}"); + self.delete(&url).await + } + + async fn update_primary_email(&self, user_id: i32, email_id: i32) -> Response<()> { + let url = format!("/api/v1/users/{user_id}/emails/{email_id}/set_primary"); + self.put(&url, "").await + } +} + +impl MockEmailHelper for crate::tests::util::MockCookieUser {} +impl MockEmailHelper for crate::tests::util::MockAnonymousUser {} + +/// Given a crates.io user, check that the user can add an email address +/// to their profile, and that the email address is then returned by the +/// `/me` endpoint. +#[tokio::test(flavor = "multi_thread")] +async fn test_email_add() -> anyhow::Result<()> { + let (_app, _anon, user) = TestApp::init().with_user().await; + + let json = user.show_me().await; + assert_eq!(json.user.emails.len(), 1); + assert_eq!(json.user.emails.first().unwrap().email, "foo@example.com"); + + let response = user.add_email(json.user.id, "bar@example.com").await; + let json = user.show_me().await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"id":2,"email":"bar@example.com","verified":false,"verification_email_sent":true,"primary":false}"#); + assert_eq!(json.user.emails.len(), 2); + assert!( + json.user + .emails + .iter() + .any(|e| e.email == "bar@example.com") + ); + assert!( + json.user + .emails + .iter() + .find(|e| e.email == "foo@example.com") + .unwrap() + .primary + ); + + Ok(()) +} + +/// Given a crates.io user, check to make sure that the user +/// cannot add to the database an empty string or null as +/// their email. If an attempt is made, the emails controller +/// will return an error indicating that an empty email cannot be +/// added. +/// +/// This is checked on the frontend already, but I'd like to +/// make sure that a user cannot get around that and delete +/// their email by adding an empty string. +#[tokio::test(flavor = "multi_thread")] +async fn test_empty_email_not_added() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let response = user.add_email(model.id, "").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"empty email rejected"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_ignore_empty_json() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let url = format!("/api/v1/users/{}/emails", model.id); + let payload = json!({}); + let response = user.post::<()>(&url, payload.to_string()).await; + assert_snapshot!(response.status(), @"422 Unprocessable Entity"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_ignore_null_email() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let url = format!("/api/v1/users/{}/emails", model.id); + let payload = json!({ "email": null }); + let response = user.post::<()>(&url, payload.to_string()).await; + assert_snapshot!(response.status(), @"422 Unprocessable Entity"); +} + +/// Check to make sure that neither other signed in users nor anonymous users can add an +/// email address to another user's account. +/// +/// If an attempt is made, the emails controller will return an error indicating that the +/// current user does not match the requested user. +#[tokio::test(flavor = "multi_thread")] +async fn test_other_users_cannot_change_my_email() { + let (app, anon, user) = TestApp::init().with_user().await; + let another_user = app.db_new_user("not_me").await; + let another_user_model = another_user.as_model(); + + let response = user + .add_email(another_user_model.id, "pineapple@pineapples.pineapple") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"current user does not match requested user"}]}"#); + + let response = anon + .add_email(another_user_model.id, "pineapple@pineapples.pineapple") + .await; + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"this action requires authentication"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_invalid_email_address() { + let (_app, _, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let response = user.add_email(model.id, "foo").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid email address"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_invalid_json() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let url = format!("/api/v1/users/{}/emails", model.id); + let response = user.post::<()>(&url, r#"{ "user": foo }"#).await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Failed to parse the request body as JSON: user: expected ident at line 1 column 12"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_delete_email_invalid_id() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let response = user.delete_email(model.id, 0).await; + assert_snapshot!(response.status(), @"404 Not Found"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Not Found"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_other_users_cannot_delete_my_email() { + let (app, anon, user) = TestApp::init().with_user().await; + let another_user = app.db_new_user("not_me").await; + let another_user_model = another_user.as_model(); + + let response = user.delete_email(another_user_model.id, 0).await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"current user does not match requested user"}]}"#); + + let response = anon.delete_email(another_user_model.id, 0).await; + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"this action requires authentication"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_cannot_delete_my_primary_email() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + // Attempt to delete the primary email address + let response = user.delete_email(model.id, 1).await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"cannot delete primary email, please set another email as primary first"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_can_delete_an_alternative_email() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + // Add an alternative email address + let response = user.add_email(model.id, "potato3@example.com").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"id":2,"email":"potato3@example.com","verified":false,"verification_email_sent":true,"primary":false}"#); + + // Attempt to delete the alternative email address + let response = user.delete_email(model.id, 2).await; + assert_snapshot!(response.status(), @"200 OK"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_set_primary_invalid_id() { + let (_app, _anon, user) = TestApp::init().with_user().await; + let model = user.as_model(); + + let response = user.update_primary_email(model.id, 0).await; + assert_snapshot!(response.status(), @"404 Not Found"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Not Found"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_other_users_cannot_set_my_primary_email() { + let (app, anon, user) = TestApp::init().with_user().await; + let another_user = app.db_new_user("not_me").await; + let another_user_model = another_user.as_model(); + + let response = user.update_primary_email(another_user_model.id, 1).await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"current user does not match requested user"}]}"#); + + let response = anon.update_primary_email(another_user_model.id, 1).await; + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"this action requires authentication"}]}"#); +} diff --git a/src/tests/routes/users/mod.rs b/src/tests/routes/users/mod.rs index c788314a57b..5e1bfc679a2 100644 --- a/src/tests/routes/users/mod.rs +++ b/src/tests/routes/users/mod.rs @@ -1,3 +1,5 @@ +mod email_verification; +mod emails; mod read; mod stats; pub mod update; diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__email_verification__happy_path-5.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__email_verification__happy_path-5.snap new file mode 100644 index 00000000000..4d28401107e --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__email_verification__happy_path-5.snap @@ -0,0 +1,40 @@ +--- +source: src/tests/routes/users/email_verification.rs +expression: app.emails_snapshot().await +--- +To: user@example.com +From: crates.io +Subject: crates.io: Please confirm your email address +Content-Type: text/plain; charset=utf-8 +Content-Transfer-Encoding: quoted-printable + + +Hello foo! + +Welcome to crates.io. Please click the link below to verify your email address: + +https://crates.io/confirm/[confirm-token] + +Thank you! + +-- +The crates.io Team +---------------------------------------- + +To: user@example.com +From: crates.io +Subject: crates.io: Please confirm your email address +Content-Type: text/plain; charset=utf-8 +Content-Transfer-Encoding: quoted-printable + + +Hello foo! + +Welcome to crates.io. Please click the link below to verify your email address: + +https://crates.io/confirm/[confirm-token] + +Thank you! + +-- +The crates.io Team diff --git a/src/tests/routes/users/update.rs b/src/tests/routes/users/update.rs index 884b1eddab6..ff72ba50f78 100644 --- a/src/tests/routes/users/update.rs +++ b/src/tests/routes/users/update.rs @@ -1,3 +1,5 @@ +//! This file tests the legacy email update functionality. + use crate::tests::util::{RequestHelper, Response, TestApp}; use http::StatusCode; use insta::assert_snapshot; diff --git a/src/tests/snapshots/crates_io__tests__user__confirm_email-2.snap b/src/tests/snapshots/crates_io__tests__user__confirm_email-2.snap new file mode 100644 index 00000000000..0eb69a21a5e --- /dev/null +++ b/src/tests/snapshots/crates_io__tests__user__confirm_email-2.snap @@ -0,0 +1,14 @@ +--- +source: src/tests/user.rs +expression: response.json() +--- +{ + "email": { + "email": "potato2@example.com", + "id": 1, + "primary": true, + "verification_email_sent": true, + "verified": true + }, + "ok": true +} diff --git a/src/tests/snapshots/crates_io__tests__user__email_legacy_get_and_put.snap b/src/tests/snapshots/crates_io__tests__user__email_legacy_get_and_put.snap new file mode 100644 index 00000000000..e3b739d2aa7 --- /dev/null +++ b/src/tests/snapshots/crates_io__tests__user__email_legacy_get_and_put.snap @@ -0,0 +1,32 @@ +--- +source: src/tests/user.rs +expression: json.user +--- +{ + "id": 1, + "login": "foo", + "emails": [ + { + "id": 1, + "email": "foo@example.com", + "verified": true, + "verification_email_sent": true, + "primary": true + }, + { + "id": 2, + "email": "mango@mangos.mango", + "verified": false, + "verification_email_sent": true, + "primary": false + } + ], + "name": null, + "email_verified": true, + "email_verification_sent": true, + "email": "foo@example.com", + "avatar": null, + "url": "https://github.com/foo", + "is_admin": false, + "publish_notifications": true +} diff --git a/src/tests/snapshots/crates_io__tests__user__initial_github_login_succeeds.snap b/src/tests/snapshots/crates_io__tests__user__initial_github_login_succeeds.snap new file mode 100644 index 00000000000..825ccd95b3c --- /dev/null +++ b/src/tests/snapshots/crates_io__tests__user__initial_github_login_succeeds.snap @@ -0,0 +1,32 @@ +--- +source: src/tests/user.rs +expression: json.user +--- +{ + "id": 1, + "login": "arbitrary_username", + "emails": [ + { + "id": 1, + "email": "foo@example.com", + "verified": true, + "verification_email_sent": true, + "primary": true + }, + { + "id": 2, + "email": "bar@example.com", + "verified": false, + "verification_email_sent": true, + "primary": false + } + ], + "name": null, + "email_verified": true, + "email_verification_sent": true, + "email": "foo@example.com", + "avatar": null, + "url": "https://github.com/arbitrary_username", + "is_admin": false, + "publish_notifications": true +} diff --git a/src/tests/user.rs b/src/tests/user.rs index ac44af2038d..d9a20883d83 100644 --- a/src/tests/user.rs +++ b/src/tests/user.rs @@ -6,19 +6,18 @@ use crate::tests::util::{MockCookieUser, RequestHelper}; use crate::util::token::HashedToken; use chrono::{DateTime, Utc}; use claims::assert_ok; -use crates_io_github::GitHubUser; +use crates_io_github::{GitHubEmail, GitHubUser}; use diesel::prelude::*; use diesel_async::RunQueryDsl; -use insta::assert_snapshot; +use insta::{assert_json_snapshot, assert_snapshot}; use secrecy::ExposeSecret; -use serde_json::json; impl crate::tests::util::MockCookieUser { async fn confirm_email(&self, email_token: &str) { let url = format!("/api/v1/confirm/{email_token}"); let response = self.put::<()>(&url, &[] as &[u8]).await; assert_snapshot!(response.status(), @"200 OK"); - assert_eq!(response.json(), json!({ "ok": true })); + assert_json_snapshot!(response.json()); } } @@ -38,7 +37,9 @@ async fn updating_existing_user_doesnt_change_api_token() -> anyhow::Result<()> email: None, avatar_url: None, }; - assert_ok!(session::save_user_to_database(&gh_user, "bar_token", emails, &mut conn).await); + assert_ok!( + session::save_user_to_database(&gh_user, &mut [], "bar_token", emails, &mut conn).await + ); // Use the original API token to find the now updated user let hashed_token = assert_ok!(HashedToken::parse(token)); @@ -51,6 +52,50 @@ async fn updating_existing_user_doesnt_change_api_token() -> anyhow::Result<()> Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn initial_github_login_succeeds() -> anyhow::Result<()> { + let (app, _) = TestApp::init().empty().await; + let emails = &app.as_inner().emails; + let mut conn = app.db_conn().await; + + // Simulate logging in via GitHub + let gh_id = next_gh_id(); + let gh_user = GitHubUser { + id: gh_id, + login: "arbitrary_username".to_string(), + name: None, + email: Some("foo@example.com".to_string()), + avatar_url: None, + }; + // The primary email is not first in the array to validate that the order does not matter + let mut gh_emails = vec![ + GitHubEmail { + email: "bar@example.com".to_string(), + verified: false, + primary: false, + }, + GitHubEmail { + email: gh_user.email.clone().unwrap(), + verified: true, + primary: true, + }, + ]; + let u = session::save_user_to_database( + &gh_user, + &mut gh_emails, + "some random token", + emails, + &mut conn, + ) + .await?; + + let user = MockCookieUser::new(&app, u); + let json = user.show_me().await; + assert_json_snapshot!(json.user); + + Ok(()) +} + /// Given a GitHub user, check that if the user logs in, /// updates their email, logs out, then logs back in, the /// email they added to crates.io will not be overwritten @@ -61,6 +106,7 @@ async fn updating_existing_user_doesnt_change_api_token() -> anyhow::Result<()> /// send none as the email and we will end up inadvertently /// deleting their email when they sign back in. #[tokio::test(flavor = "multi_thread")] +#[allow(deprecated)] async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> { let (app, _) = TestApp::init().empty().await; let emails = &app.as_inner().emails; @@ -80,13 +126,14 @@ async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> { }; let u = - session::save_user_to_database(&gh_user, "some random token", emails, &mut conn).await?; + session::save_user_to_database(&gh_user, &mut [], "some random token", emails, &mut conn) + .await?; let user_without_github_email = MockCookieUser::new(&app, u); 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.emails.len(), 0); // Add an email address in crates.io user_without_github_email @@ -104,19 +151,23 @@ async fn github_without_email_does_not_overwrite_email() -> anyhow::Result<()> { }; let u = - session::save_user_to_database(&gh_user, "some random token", emails, &mut conn).await?; + session::save_user_to_database(&gh_user, &mut [], "some random token", emails, &mut conn) + .await?; 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.emails[0].email, "apricot@apricots.apricot"); + assert_eq!(json.user.primary_email.unwrap(), "apricot@apricots.apricot"); Ok(()) } /// Given a new user, test that if they sign in with one email, change their email on GitHub, then -/// sign in again, that the email in crates.io will remain set to the original email used on GitHub. +/// sign in again, that both emails will be present on their crates.io account, with the original +/// remaining as the primary email. #[tokio::test(flavor = "multi_thread")] +#[allow(deprecated)] async fn github_with_email_does_not_overwrite_email() -> anyhow::Result<()> { use crate::schema::emails; @@ -144,34 +195,54 @@ async fn github_with_email_does_not_overwrite_email() -> anyhow::Result<()> { email: Some(new_github_email.to_string()), avatar_url: None, }; + let gh_email = GitHubEmail { + email: gh_user.email.clone().unwrap_or_default(), + verified: true, + primary: true, + }; - let u = - session::save_user_to_database(&gh_user, "some random token", &emails, &mut conn).await?; + let u = session::save_user_to_database( + &gh_user, + &mut [gh_email], + "some random token", + &emails, + &mut conn, + ) + .await?; 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!(json.user.emails.iter().any(|e| e.email == new_github_email)); + assert!( + json.user + .emails + .iter() + .find(|e| e.email == original_email) + .unwrap() + .primary + ); + assert_eq!(json.user.primary_email, Some(original_email)); Ok(()) } /// Given a crates.io user, check that the user's email can be -/// updated in the database (PUT /user/{user_id}), then check -/// that the updated email is sent back to the user (GET /me). +/// updated in the database using the legacy endpoint +/// (PUT /user/{user_id}), then check that the updated email +/// is included in the user's email list (GET /me). #[tokio::test(flavor = "multi_thread")] -async fn test_email_get_and_put() -> anyhow::Result<()> { +#[allow(deprecated)] +async fn test_email_legacy_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_json_snapshot!(json.user); Ok(()) } @@ -182,6 +253,7 @@ async fn test_email_get_and_put() -> anyhow::Result<()> { /// requested, check that the response back is ok, and that /// the email_verified field on user is now set to true. #[tokio::test(flavor = "multi_thread")] +#[allow(deprecated)] async fn test_confirm_user_email() -> anyhow::Result<()> { use crate::schema::emails; @@ -201,9 +273,20 @@ async fn test_confirm_user_email() -> anyhow::Result<()> { email: Some(email.to_string()), avatar_url: None, }; + let gh_email = GitHubEmail { + email: gh_user.email.clone().unwrap_or_default(), + verified: true, + primary: true, + }; - let u = - session::save_user_to_database(&gh_user, "some random token", emails, &mut conn).await?; + let u = session::save_user_to_database( + &gh_user, + &mut [gh_email], + "some random token", + emails, + &mut conn, + ) + .await?; let user = MockCookieUser::new(&app, u); let user_model = user.as_model(); @@ -216,9 +299,66 @@ 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); + + // Check emails array + assert_eq!(json.user.emails.len(), 1); + assert_eq!(json.user.emails[0].email, "potato2@example.com"); + assert!(json.user.emails[0].verified); + assert!(json.user.emails[0].primary); + + // Check legacy fields + assert_eq!(json.user.primary_email.unwrap(), "potato2@example.com"); + assert!(json.user.primary_email_verified); + assert!(json.user.primary_email_verification_sent); + + Ok(()) +} + +/// Given a new user, who has a single email address on GitHub +/// which is not verified, check that their email is not marked +/// as verified in the database, and that a verification email +/// is sent to the user. +#[tokio::test(flavor = "multi_thread")] +#[allow(deprecated)] +async fn test_unverified_email_not_marked_verified() -> anyhow::Result<()> { + let (app, _) = TestApp::init().empty().await; + let mut conn = app.db_conn().await; + + let email = "potato3@example.com"; + let emails = &app.as_inner().emails; + let gh_user = GitHubUser { + id: next_gh_id(), + login: "arbitrary_username".to_string(), + name: None, + email: Some(email.to_string()), + avatar_url: None, + }; + let gh_email = GitHubEmail { + email: gh_user.email.clone().unwrap_or_default(), + verified: false, + primary: true, + }; + let u = session::save_user_to_database( + &gh_user, + &mut [gh_email], + "some + random token", + emails, + &mut conn, + ) + .await?; + let user = MockCookieUser::new(&app, u); + let json = user.show_me().await; + // Check emails array + assert_eq!(json.user.emails.len(), 1); + assert_eq!(json.user.emails[0].email, "potato3@example.com"); + assert!(!json.user.emails[0].verified); + assert!(json.user.emails[0].primary); + + // Check legacy fields + assert_eq!(json.user.primary_email.unwrap(), "potato3@example.com"); + assert!(!json.user.primary_email_verified); + assert!(json.user.primary_email_verification_sent); Ok(()) } @@ -227,6 +367,7 @@ async fn test_confirm_user_email() -> anyhow::Result<()> { /// test that `email_verification_sent` is false so that we don't /// make the user think we've sent an email when we haven't. #[tokio::test(flavor = "multi_thread")] +#[allow(deprecated)] async fn test_existing_user_email() -> anyhow::Result<()> { use crate::schema::emails; use diesel::update; @@ -247,9 +388,20 @@ async fn test_existing_user_email() -> anyhow::Result<()> { email: Some(email.to_string()), avatar_url: None, }; + let gh_email = GitHubEmail { + email: gh_user.email.clone().unwrap_or_default(), + verified: false, + primary: true, + }; - let u = - session::save_user_to_database(&gh_user, "some random token", emails, &mut conn).await?; + let u = session::save_user_to_database( + &gh_user, + &mut [gh_email], + "some random token", + emails, + &mut conn, + ) + .await?; update(Email::belonging_to(&u)) // Users created before we added verification will have @@ -260,9 +412,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.rs b/src/tests/util.rs index 6b6bfae932f..0476078368d 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -316,6 +316,23 @@ impl MockCookieUser { &self.user } + /// Creates an email for the user, directly inserting it into the database + pub async fn db_new_email( + &self, + email: &str, + verified: bool, + primary: bool, + ) -> crate::models::Email { + let mut conn = self.app.db_conn().await; + let new_email = crate::models::NewEmail::builder() + .user_id(self.user.id) + .email(email) + .verified(verified) + .primary(primary) + .build(); + new_email.insert(&mut conn).await.unwrap() + } + /// Creates a token and wraps it in a helper struct /// /// This method updates the database directly 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..cc0ed01c017 100644 --- a/src/views.rs +++ b/src/views.rs @@ -1,7 +1,8 @@ use crate::external_urls::remove_blocked_urls; use crate::models::{ - ApiToken, Category, Crate, Dependency, DependencyKind, Keyword, Owner, ReverseDependency, Team, - TopVersions, TrustpubData, User, Version, VersionDownload, VersionOwnerAction, + ApiToken, Category, Crate, Dependency, DependencyKind, Email, Keyword, Owner, + ReverseDependency, Team, TopVersions, TrustpubData, User, Version, VersionDownload, + VersionOwnerAction, }; use chrono::{DateTime, Utc}; use crates_io_github as github; @@ -676,21 +677,42 @@ 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 email addresses. + #[schema(example = json!([ + { + "id": 42, + "email": "user@example.com", + "verified": true, + "primary": true, + "verification_email_sent": true + }]))] + pub emails: Vec, /// 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")] + #[deprecated(note = "Use `emails` array instead, check that `verified` property is true.")] + 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")] + #[deprecated( + note = "Use `emails` array instead, check that `token_generated_at` property is not null." + )] + 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")] + #[deprecated( + note = "Use `emails` array instead, maximum of one entry will have `primary` property set to true." + )] + pub primary_email: Option, /// The user's avatar URL, if set. #[schema(example = "https://avatars2.githubusercontent.com/u/1234567?v=4")] @@ -711,12 +733,7 @@ pub struct EncodablePrivateUser { 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, - ) -> Self { + pub fn from(user: User, emails: Vec) -> Self { let User { id, name, @@ -728,11 +745,19 @@ impl EncodablePrivateUser { } = user; let url = format!("https://github.com/{gh_login}"); + let primary_email = emails.iter().find(|e| e.primary); + let primary_email_verified = primary_email.map(|e| e.verified).unwrap_or(false); + let primary_email_verification_sent = + primary_email.and_then(|e| e.token_generated_at).is_some(); + let primary_email = primary_email.map(|e| e.email.clone()); + + #[allow(deprecated)] EncodablePrivateUser { id, - email, - email_verified, - email_verification_sent, + emails: emails.into_iter().map(EncodableEmail::from).collect(), + primary_email_verified, + primary_email_verification_sent, + primary_email, avatar: gh_avatar, login: gh_login, name, @@ -743,6 +768,42 @@ impl EncodablePrivateUser { } } +#[derive(Deserialize, Serialize, Debug, utoipa::ToSchema)] +#[schema(as = Email)] +pub struct EncodableEmail { + /// An opaque identifier for the email. + #[schema(example = 42)] + pub id: i32, + + /// The email address. + #[schema(example = "user@example.com")] + pub email: String, + + /// Whether the email address has been verified. + #[schema(example = true)] + pub verified: bool, + + /// Whether the verification email has been sent. + #[schema(example = true)] + pub verification_email_sent: bool, + + /// Whether this is the user's primary email address, meaning notifications will be sent here. + #[schema(example = true)] + pub primary: bool, +} + +impl From for EncodableEmail { + fn from(email: Email) -> Self { + Self { + id: email.id, + email: email.email, + verified: email.verified, + verification_email_sent: email.token_generated_at.is_some(), + primary: email.primary, + } + } +} + #[derive(Deserialize, Serialize, Debug, PartialEq, Eq, utoipa::ToSchema)] #[schema(as = User)] pub struct EncodablePublicUser { 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?; diff --git a/tests/acceptance/email-change-test.js b/tests/acceptance/email-change-test.js deleted file mode 100644 index 285b35ecc41..00000000000 --- a/tests/acceptance/email-change-test.js +++ /dev/null @@ -1,177 +0,0 @@ -import { click, currentURL, fillIn } from '@ember/test-helpers'; -import { module, test } from 'qunit'; - -import { http, HttpResponse } from 'msw'; - -import { setupApplicationTest } from 'crates-io/tests/helpers'; - -import { visit } from '../helpers/visit-ignoring-abort'; - -module('Acceptance | Email Change', function (hooks) { - setupApplicationTest(hooks); - - test('happy path', async function (assert) { - let user = this.db.user.create({ email: 'old@email.com' }); - - this.authenticateAs(user); - - await visit('/settings/profile'); - assert.strictEqual(currentURL(), '/settings/profile'); - assert.dom('[data-test-email-input]').exists(); - assert.dom('[data-test-email-input] [data-test-no-email]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-email-address]').includesText('old@email.com'); - assert.dom('[data-test-email-input] [data-test-verified]').exists(); - assert.dom('[data-test-email-input] [data-test-not-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-resend-button]').doesNotExist(); - - await click('[data-test-email-input] [data-test-edit-button]'); - assert.dom('[data-test-email-input] [data-test-input]').hasValue('old@email.com'); - assert.dom('[data-test-email-input] [data-test-save-button]').isEnabled(); - assert.dom('[data-test-email-input] [data-test-cancel-button]').isEnabled(); - - await fillIn('[data-test-email-input] [data-test-input]', ''); - assert.dom('[data-test-email-input] [data-test-input]').hasValue(''); - assert.dom('[data-test-email-input] [data-test-save-button]').isDisabled(); - - await fillIn('[data-test-email-input] [data-test-input]', 'new@email.com'); - assert.dom('[data-test-email-input] [data-test-input]').hasValue('new@email.com'); - assert.dom('[data-test-email-input] [data-test-save-button]').isEnabled(); - - await click('[data-test-email-input] [data-test-save-button]'); - assert.dom('[data-test-email-input] [data-test-email-address]').includesText('new@email.com'); - assert.dom('[data-test-email-input] [data-test-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-not-verified]').exists(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').exists(); - assert.dom('[data-test-email-input] [data-test-resend-button]').isEnabled(); - - user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); - assert.strictEqual(user.email, 'new@email.com'); - assert.false(user.emailVerified); - assert.ok(user.emailVerificationToken); - }); - - test('happy path with `email: null`', async function (assert) { - let user = this.db.user.create({ email: undefined }); - - this.authenticateAs(user); - - await visit('/settings/profile'); - assert.strictEqual(currentURL(), '/settings/profile'); - assert.dom('[data-test-email-input]').exists(); - assert.dom('[data-test-email-input] [data-test-no-email]').exists(); - assert.dom('[data-test-email-input] [data-test-email-address]').hasText(''); - assert.dom('[data-test-email-input] [data-test-not-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-resend-button]').doesNotExist(); - - await click('[data-test-email-input] [data-test-edit-button]'); - assert.dom('[data-test-email-input] [data-test-input]').hasValue(''); - assert.dom('[data-test-email-input] [data-test-save-button]').isDisabled(); - assert.dom('[data-test-email-input] [data-test-cancel-button]').isEnabled(); - - await fillIn('[data-test-email-input] [data-test-input]', 'new@email.com'); - assert.dom('[data-test-email-input] [data-test-input]').hasValue('new@email.com'); - assert.dom('[data-test-email-input] [data-test-save-button]').isEnabled(); - - await click('[data-test-email-input] [data-test-save-button]'); - assert.dom('[data-test-email-input] [data-test-no-email]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-email-address]').includesText('new@email.com'); - assert.dom('[data-test-email-input] [data-test-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-not-verified]').exists(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').exists(); - assert.dom('[data-test-email-input] [data-test-resend-button]').isEnabled(); - - user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); - assert.strictEqual(user.email, 'new@email.com'); - assert.false(user.emailVerified); - assert.ok(user.emailVerificationToken); - }); - - test('cancel button', async function (assert) { - let user = this.db.user.create({ email: 'old@email.com' }); - - this.authenticateAs(user); - - await visit('/settings/profile'); - await click('[data-test-email-input] [data-test-edit-button]'); - await fillIn('[data-test-email-input] [data-test-input]', 'new@email.com'); - assert.dom('[data-test-email-input] [data-test-invalid-email-warning]').doesNotExist(); - - await click('[data-test-email-input] [data-test-cancel-button]'); - assert.dom('[data-test-email-input] [data-test-email-address]').includesText('old@email.com'); - assert.dom('[data-test-email-input] [data-test-verified]').exists(); - assert.dom('[data-test-email-input] [data-test-not-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').doesNotExist(); - - user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); - assert.strictEqual(user.email, 'old@email.com'); - assert.true(user.emailVerified); - assert.notOk(user.emailVerificationToken); - }); - - test('server error', async function (assert) { - let user = this.db.user.create({ email: 'old@email.com' }); - - this.authenticateAs(user); - - this.worker.use(http.put('/api/v1/users/:user_id', () => HttpResponse.json({}, { status: 500 }))); - - await visit('/settings/profile'); - await click('[data-test-email-input] [data-test-edit-button]'); - await fillIn('[data-test-email-input] [data-test-input]', 'new@email.com'); - - await click('[data-test-email-input] [data-test-save-button]'); - assert.dom('[data-test-email-input] [data-test-input]').hasValue('new@email.com'); - assert.dom('[data-test-email-input] [data-test-email-address]').doesNotExist(); - assert - .dom('[data-test-notification-message="error"]') - .hasText('Error in saving email: An unknown error occurred while saving this email.'); - - user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); - assert.strictEqual(user.email, 'old@email.com'); - assert.true(user.emailVerified); - assert.notOk(user.emailVerificationToken); - }); - - module('Resend button', function () { - test('happy path', async function (assert) { - let user = this.db.user.create({ email: 'john@doe.com', emailVerificationToken: 'secret123' }); - - this.authenticateAs(user); - - await visit('/settings/profile'); - assert.strictEqual(currentURL(), '/settings/profile'); - assert.dom('[data-test-email-input]').exists(); - assert.dom('[data-test-email-input] [data-test-email-address]').includesText('john@doe.com'); - assert.dom('[data-test-email-input] [data-test-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-not-verified]').exists(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').exists(); - assert.dom('[data-test-email-input] [data-test-resend-button]').isEnabled().hasText('Resend'); - - await click('[data-test-email-input] [data-test-resend-button]'); - assert.dom('[data-test-email-input] [data-test-resend-button]').isDisabled().hasText('Sent!'); - }); - - test('server error', async function (assert) { - let user = this.db.user.create({ email: 'john@doe.com', emailVerificationToken: 'secret123' }); - - this.authenticateAs(user); - - this.worker.use(http.put('/api/v1/users/:user_id/resend', () => HttpResponse.json({}, { status: 500 }))); - - await visit('/settings/profile'); - assert.strictEqual(currentURL(), '/settings/profile'); - assert.dom('[data-test-email-input]').exists(); - assert.dom('[data-test-email-input] [data-test-email-address]').includesText('john@doe.com'); - assert.dom('[data-test-email-input] [data-test-verified]').doesNotExist(); - assert.dom('[data-test-email-input] [data-test-not-verified]').exists(); - assert.dom('[data-test-email-input] [data-test-verification-sent]').exists(); - assert.dom('[data-test-email-input] [data-test-resend-button]').isEnabled().hasText('Resend'); - - await click('[data-test-email-input] [data-test-resend-button]'); - assert.dom('[data-test-email-input] [data-test-resend-button]').isEnabled().hasText('Resend'); - assert.dom('[data-test-notification-message="error"]').hasText('Unknown error in resending message'); - }); - }); -}); diff --git a/tests/acceptance/email-confirmation-test.js b/tests/acceptance/email-confirmation-test.js index 00a6f386a64..9d15a473a5b 100644 --- a/tests/acceptance/email-confirmation-test.js +++ b/tests/acceptance/email-confirmation-test.js @@ -9,20 +9,21 @@ module('Acceptance | Email Confirmation', function (hooks) { setupApplicationTest(hooks); test('unauthenticated happy path', async function (assert) { - let user = this.db.user.create({ emailVerificationToken: 'badc0ffee' }); - assert.false(user.emailVerified); + let email = this.db.email.create({ verified: false, token: 'badc0ffee' }); + let user = this.db.user.create({ emails: [email] }); + assert.false(email.verified); await visit('/confirm/badc0ffee'); assert.strictEqual(currentURL(), '/'); assert.dom('[data-test-notification-message="success"]').exists(); user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); - assert.true(user.emailVerified); + assert.true(user.emails[0].verified); }); test('authenticated happy path', async function (assert) { - let user = this.db.user.create({ emailVerificationToken: 'badc0ffee' }); - assert.false(user.emailVerified); + let user = this.db.user.create({ emails: [this.db.email.create({ verified: false, token: 'badc0ffee' })] }); + assert.false(user.emails[0].verified); this.authenticateAs(user); @@ -31,10 +32,10 @@ module('Acceptance | Email Confirmation', function (hooks) { assert.dom('[data-test-notification-message="success"]').exists(); let { currentUser } = this.owner.lookup('service:session'); - assert.true(currentUser.email_verified); + assert.true(currentUser.emails[0].verified); user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); - assert.true(user.emailVerified); + assert.true(user.emails[0].verified); }); test('error case', async function (assert) { diff --git a/tests/acceptance/email-test.js b/tests/acceptance/email-test.js new file mode 100644 index 00000000000..d130de85823 --- /dev/null +++ b/tests/acceptance/email-test.js @@ -0,0 +1,248 @@ +import { click, currentURL, fillIn } from '@ember/test-helpers'; +import { module, test } from 'qunit'; + +import { http, HttpResponse } from 'msw'; + +import { setupApplicationTest } from 'crates-io/tests/helpers'; + +import { visit } from '../helpers/visit-ignoring-abort'; + +module('Acceptance | Email Management', function (hooks) { + setupApplicationTest(hooks); + + module('Add email', function () { + test('happy path', async function (assert) { + let user = this.db.user.create({ emails: [this.db.email.create({ email: 'old@email.com' })] }); + assert.strictEqual(user.emails[0].email, 'old@email.com'); + assert.false(user.emails[0].verified); + + this.authenticateAs(user); + + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-add-email-button]').exists(); + assert.dom('[data-test-add-email-input]').doesNotExist(); + + await click('[data-test-add-email-button]'); + assert.dom('[data-test-add-email-input]').exists(); + assert.dom('[data-test-add-email-input] [data-test-unverified]').doesNotExist(); + assert.dom('[data-test-add-email-input] [data-test-verified]').doesNotExist(); + assert.dom('[data-test-add-email-input] [data-test-verification-sent]').doesNotExist(); + assert.dom('[data-test-add-email-input] [data-test-resend-button]').doesNotExist(); + + await fillIn('[data-test-add-email-input] [data-test-input]', ''); + assert.dom('[data-test-add-email-input] [data-test-input]').hasValue(''); + assert.dom('[data-test-add-email-input] [data-test-save-button]').isDisabled(); + + await fillIn('[data-test-add-email-input] [data-test-input]', 'notanemail'); + assert.dom('[data-test-add-email-input] [data-test-input]').hasValue('notanemail'); + assert.dom('[data-test-add-email-input] [data-test-save-button]').isDisabled(); + + await fillIn('[data-test-add-email-input] [data-test-input]', 'new@email.com'); + assert.dom('[data-test-add-email-input] [data-test-input]').hasValue('new@email.com'); + assert.dom('[data-test-add-email-input] [data-test-save-button]').isEnabled(); + + await click('[data-test-add-email-input] [data-test-save-button]'); + assert.dom('[data-test-add-email-button]').exists(); + assert.dom('[data-test-add-email-input]').doesNotExist(); + + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('old@email.com'); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-email-address]').includesText('new@email.com'); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-verified]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-unverified]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-verification-sent]').exists(); + + user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); + assert.strictEqual(user.emails[0].email, 'old@email.com'); + assert.strictEqual(user.emails[1].email, 'new@email.com'); + assert.false(user.emails[1].verified); + }); + + test('happy path with no previous emails', async function (assert) { + let user = this.db.user.create({ emails: [] }); + assert.strictEqual(user.emails.length, 0); + + this.authenticateAs(user); + + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-add-email-button]').exists(); + assert.dom('[data-test-add-email-input]').doesNotExist(); + + await click('[data-test-add-email-button]'); + assert.dom('[data-test-add-email-input]').exists(); + + await fillIn('[data-test-add-email-input] [data-test-input]', 'new@email.com'); + await click('[data-test-add-email-input] [data-test-save-button]'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('new@email.com'); + + user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); + assert.strictEqual(user.emails.length, 1); + assert.strictEqual(user.emails[0].email, 'new@email.com'); + }); + + test('server error', async function (assert) { + let user = this.db.user.create({ emails: [this.db.email.create({ email: 'old@email.com' })] }); + + this.authenticateAs(user); + + this.worker.use(http.post('/api/v1/users/:user_id/emails', () => HttpResponse.json({}, { status: 500 }))); + + await visit('/settings/profile'); + await click('[data-test-add-email-button]'); + assert.dom('[data-test-add-email-input]').exists(); + + await fillIn('[data-test-add-email-input] [data-test-input]', 'new@email.com'); + await click('[data-test-add-email-input] [data-test-save-button]'); + assert.dom('[data-test-notification-message="error"]').hasText('Unknown error in saving email'); + + user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); + assert.strictEqual(user.emails[0].email, 'old@email.com'); + assert.strictEqual(user.emails.length, 1); + }); + }); + + module('Remove email', function () { + test('happy path', async function (assert) { + let user = this.db.user.create({ + emails: [this.db.email.create({ email: 'john@doe.com' }), this.db.email.create({ email: 'jane@doe.com' })], + }); + + this.authenticateAs(user); + + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('john@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-email-address]').includesText('jane@doe.com'); + + await click('[data-test-email-input]:nth-of-type(2) [data-test-remove-button]'); + assert.dom('[data-test-email-input]').exists({ count: 1 }); + assert.dom('[data-test-email-input] [data-test-remove-button]').doesNotExist(); + + user = this.db.user.findFirst({ where: { id: { equals: user.id } } }); + assert.strictEqual(user.emails[0].email, 'john@doe.com'); + assert.strictEqual(user.emails.length, 1); + }); + + test('cannot remove primary email', async function (assert) { + let user = this.db.user.create({ + emails: [ + this.db.email.create({ email: 'primary@doe.com', primary: true }), + this.db.email.create({ email: 'john@doe.com' }), + ], + }); + this.authenticateAs(user); + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('primary@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-remove-button]').isDisabled(); + assert + .dom('[data-test-email-input]:nth-of-type(1) [data-test-remove-button]') + .hasAttribute('title', 'Cannot delete primary email'); + }); + + test('no delete button when only one email', async function (assert) { + let user = this.db.user.create({ emails: [this.db.email.create({ email: 'john@doe.com' })] }); + this.authenticateAs(user); + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('john@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-remove-button]').doesNotExist(); + }); + + test('server error', async function (assert) { + let user = this.db.user.create({ + emails: [this.db.email.create({ email: 'john@doe.com' }), this.db.email.create({ email: 'jane@doe.com' })], + }); + + this.authenticateAs(user); + + this.worker.use( + http.delete('/api/v1/users/:user_id/emails/:email_id', () => HttpResponse.json({}, { status: 500 })), + ); + + await visit('/settings/profile'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('john@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-remove-button]').exists(); + await click('[data-test-email-input]:nth-of-type(1) [data-test-remove-button]'); + assert.dom('[data-test-notification-message="error"]').hasText('Unknown error in deleting email'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-remove-button]').isEnabled(); + }); + }); + + module('Resend verification email', function () { + test('happy path', async function (assert) { + let user = this.db.user.create({ + emails: [this.db.email.create({ email: 'john@doe.com', verified: false, verification_email_sent: true })], + }); + + this.authenticateAs(user); + + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-email-input]').exists(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('john@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-verified]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-unverified]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-verification-sent]').exists(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-resend-button]').isEnabled().hasText('Resend'); + + await click('[data-test-email-input] [data-test-resend-button]'); + assert.dom('[data-test-email-input] [data-test-resend-button]').isDisabled().hasText('Sent!'); + }); + + test('server error', async function (assert) { + let user = this.db.user.create({ + emails: [this.db.email.create({ email: 'john@doe.com', verified: false, verification_email_sent: true })], + }); + + this.authenticateAs(user); + + this.worker.use( + http.put('/api/v1/users/:user_id/emails/:email_id/resend', () => HttpResponse.json({}, { status: 500 })), + ); + + await visit('/settings/profile'); + assert.strictEqual(currentURL(), '/settings/profile'); + assert.dom('[data-test-email-input]').exists(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('john@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-verified]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-unverified]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-verification-sent]').exists(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-resend-button]').isEnabled().hasText('Resend'); + + await click('[data-test-email-input]:nth-of-type(1) [data-test-resend-button]'); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-resend-button]').isEnabled().hasText('Resend'); + assert.dom('[data-test-notification-message="error"]').hasText('Unknown error in resending message'); + }); + }); + + module('Switch primary email', function () { + test('happy path', async function (assert) { + let user = this.db.user.create({ + emails: [ + this.db.email.create({ email: 'john@doe.com', verified: true, primary: true }), + this.db.email.create({ email: 'jane@doe.com', verified: true, primary: false }), + ], + }); + this.authenticateAs(user); + + await visit('/settings/profile'); + + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-email-address]').includesText('john@doe.com'); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-email-address]').includesText('jane@doe.com'); + + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-primary]').isVisible(); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-primary]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-primary-button]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-primary-button]').isEnabled(); + + await click('[data-test-email-input]:nth-of-type(2) [data-test-primary-button]'); + + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-primary]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-primary]').isVisible(); + assert.dom('[data-test-email-input]:nth-of-type(2) [data-test-primary-button]').doesNotExist(); + assert.dom('[data-test-email-input]:nth-of-type(1) [data-test-primary-button]').isEnabled(); + }); + }); +}); diff --git a/tests/acceptance/publish-notifications-test.js b/tests/acceptance/publish-notifications-test.js index 383a1237478..23e554dc182 100644 --- a/tests/acceptance/publish-notifications-test.js +++ b/tests/acceptance/publish-notifications-test.js @@ -11,7 +11,7 @@ module('Acceptance | publish notifications', function (hooks) { setupApplicationTest(hooks); test('unsubscribe and resubscribe', async function (assert) { - let user = this.db.user.create(); + let user = this.db.user.create({ emails: [this.db.email.create({ verified: true })] }); this.authenticateAs(user); assert.true(user.publishNotifications); @@ -36,7 +36,7 @@ module('Acceptance | publish notifications', function (hooks) { }); test('loading and error state', async function (assert) { - let user = this.db.user.create(); + let user = this.db.user.create({ emails: [this.db.email.create({ verified: true })] }); let deferred = defer(); this.worker.use(http.put('/api/v1/users/:user_id', () => deferred.promise)); diff --git a/tests/acceptance/settings/settings-test.js b/tests/acceptance/settings/settings-test.js index cd35b7c150a..283bbe327bd 100644 --- a/tests/acceptance/settings/settings-test.js +++ b/tests/acceptance/settings/settings-test.js @@ -14,7 +14,10 @@ module('Acceptance | Settings', function (hooks) { function prepare(context) { let { db } = context; - let user1 = db.user.create({ name: 'blabaere' }); + let user1 = db.user.create({ + name: 'blabaere', + emails: [db.email.create({ email: 'blabaere@crates.io', primary: true })], + }); let user2 = db.user.create({ name: 'thehydroimpulse' }); let team1 = db.team.create({ org: 'org', name: 'blabaere' }); let team2 = db.team.create({ org: 'org', name: 'thehydroimpulse' }); diff --git a/tests/models/trustpub-github-config-test.js b/tests/models/trustpub-github-config-test.js index 249c0ee2c41..d9778324a55 100644 --- a/tests/models/trustpub-github-config-test.js +++ b/tests/models/trustpub-github-config-test.js @@ -64,7 +64,7 @@ module('Model | TrustpubGitHubConfig', function (hooks) { module('createRecord()', function () { test('creates a new GitHub config', async function (assert) { - let user = this.db.user.create({ emailVerified: true }); + let user = this.db.user.create({ emails: [this.db.email.create({ verified: true })] }); this.authenticateAs(user); let crate = this.db.crate.create(); @@ -103,7 +103,7 @@ module('Model | TrustpubGitHubConfig', function (hooks) { }); test('returns an error if the user is not an owner of the crate', async function (assert) { - let user = this.db.user.create({ emailVerified: true }); + let user = this.db.user.create({ emails: [this.db.email.create({ verified: true })] }); this.authenticateAs(user); let crate = this.db.crate.create(); @@ -123,7 +123,7 @@ module('Model | TrustpubGitHubConfig', function (hooks) { }); test('returns an error if the user does not have a verified email', async function (assert) { - let user = this.db.user.create({ emailVerified: false }); + let user = this.db.user.create({ emails: [this.db.email.create({ verified: false })] }); this.authenticateAs(user); let crate = this.db.crate.create(); diff --git a/tests/models/user-test.js b/tests/models/user-test.js index be4bd853ee7..94881637b57 100644 --- a/tests/models/user-test.js +++ b/tests/models/user-test.js @@ -13,34 +13,100 @@ module('Model | User', function (hooks) { this.store = this.owner.lookup('service:store'); }); - module('changeEmail()', function () { + module('addEmail()', function () { test('happy path', async function (assert) { - let user = this.db.user.create({ email: 'old@email.com' }); + let email = this.db.email.create({ email: 'old@email.com' }); + let user = this.db.user.create({ emails: [email] }); this.authenticateAs(user); let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); - assert.strictEqual(currentUser.email, 'old@email.com'); - assert.true(currentUser.email_verified); - assert.true(currentUser.email_verification_sent); - - await currentUser.changeEmail('new@email.com'); - assert.strictEqual(currentUser.email, 'new@email.com'); - assert.false(currentUser.email_verified); - assert.true(currentUser.email_verification_sent); + assert.strictEqual(currentUser.emails[0].email, 'old@email.com'); + + await currentUser.addEmail('new@email.com'); + assert.strictEqual(currentUser.emails[1].email, 'new@email.com'); }); test('error handling', async function (assert) { - let user = this.db.user.create({ email: 'old@email.com' }); + let email = this.db.email.create({ email: 'old@email.com' }); + let user = this.db.user.create({ emails: [email] }); this.authenticateAs(user); let error = HttpResponse.json({}, { status: 500 }); - this.worker.use(http.put('/api/v1/users/:user_id', () => error)); + this.worker.use(http.post('/api/v1/users/:user_id/emails', () => error)); + + let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); + + await assert.rejects(currentUser.addEmail('new@email.com'), function (error) { + assert.deepEqual(error.errors, [ + { + detail: '{}', + status: '500', + title: 'The backend responded with an error', + }, + ]); + return true; + }); + }); + }); + + module('deleteEmail()', function () { + test('happy path', async function (assert) { + let email = this.db.email.create({ email: 'old@email.com' }); + let user = this.db.user.create({ emails: [email] }); + this.authenticateAs(user); let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); - await assert.rejects(currentUser.changeEmail('new@email.com'), function (error) { + await currentUser.deleteEmail(email.id); + assert.false(currentUser.emails.some(e => e.id === email.id)); + }); + + test('error handling', async function (assert) { + let email = this.db.email.create({ email: 'old@email.com' }); + let user = this.db.user.create({ emails: [email] }); + this.authenticateAs(user); + + let error = HttpResponse.json({}, { status: 500 }); + this.worker.use(http.delete('/api/v1/users/:user_id/emails/:email_id', () => error)); + + let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); + + await assert.rejects(currentUser.deleteEmail(email.id), function (error) { + assert.deepEqual(error.errors, [ + { + detail: '{}', + status: '500', + title: 'The backend responded with an error', + }, + ]); + return true; + }); + }); + }); + + module('updatePrimaryEmail()', function () { + test('happy path', async function (assert) { + let email = this.db.email.create({ email: 'old@email.com' }); + let user = this.db.user.create({ emails: [email] }); + this.authenticateAs(user); + + let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); + + await currentUser.updatePrimaryEmail(email.id, 'new@email.com'); + assert.strictEqual(currentUser.emails.find(e => e.primary).id, email.id); + }); + test('error handling', async function (assert) { + let email = this.db.email.create({ email: 'old@email.com' }); + let user = this.db.user.create({ emails: [email] }); + this.authenticateAs(user); + + let error = HttpResponse.json({}, { status: 500 }); + this.worker.use(http.put('/api/v1/users/:user_id/emails/:email_id/set_primary', () => error)); + + let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); + await assert.rejects(currentUser.updatePrimaryEmail(email.id, 'new@email.com'), function (error) { assert.deepEqual(error.errors, [ { detail: '{}', @@ -57,24 +123,26 @@ module('Model | User', function (hooks) { test('happy path', async function (assert) { assert.expect(0); - let user = this.db.user.create({ emailVerificationToken: 'secret123' }); + let email = this.db.email.create({ token: 'secret123' }); + let user = this.db.user.create({ emails: [email] }); this.authenticateAs(user); let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); - await currentUser.resendVerificationEmail(); + await currentUser.resendVerificationEmail(email.id); }); test('error handling', async function (assert) { - let user = this.db.user.create({ emailVerificationToken: 'secret123' }); + let email = this.db.email.create({ token: 'secret123' }); + let user = this.db.user.create({ emails: [email] }); this.authenticateAs(user); let error = HttpResponse.json({}, { status: 500 }); - this.worker.use(http.put('/api/v1/users/:user_id/resend', () => error)); + this.worker.use(http.put('/api/v1/users/:user_id/emails/:email_id/resend', () => error)); let { currentUser } = await this.owner.lookup('service:session').loadUserTask.perform(); - await assert.rejects(currentUser.resendVerificationEmail(), function (error) { + await assert.rejects(currentUser.resendVerificationEmail(email.id), function (error) { assert.deepEqual(error.errors, [ { detail: '{}', diff --git a/tests/routes/crate/settings-test.js b/tests/routes/crate/settings-test.js index c05da7d94ff..209ff1da981 100644 --- a/tests/routes/crate/settings-test.js +++ b/tests/routes/crate/settings-test.js @@ -12,7 +12,9 @@ module('Route | crate.settings', hooks => { setupApplicationTest(hooks); function prepare(context) { - const user = context.db.user.create(); + const user = context.db.user.create({ + emails: [context.db.email.create({ email: 'user-1@crates.io', primary: true, verified: true })], + }); const crate = context.db.crate.create({ name: 'foo' }); context.db.version.create({ crate }); diff --git a/tests/routes/crate/settings/new-trusted-publisher-test.js b/tests/routes/crate/settings/new-trusted-publisher-test.js index 157231163ae..7e542499be7 100644 --- a/tests/routes/crate/settings/new-trusted-publisher-test.js +++ b/tests/routes/crate/settings/new-trusted-publisher-test.js @@ -14,7 +14,9 @@ module('Route | crate.settings.new-trusted-publisher', hooks => { setupApplicationTest(hooks); function prepare(context) { - let user = context.db.user.create(); + let user = context.db.user.create({ + emails: [context.db.email.create({ email: 'user-1@crates.io', verified: true, primary: true })], + }); let crate = context.db.crate.create({ name: 'foo' }); context.db.version.create({ crate });