From e3eda68147f76c654b54cf0a5702ef90b5dd8efe Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 21 Jun 2023 11:26:07 +0200 Subject: [PATCH] Correct error messages if user registers with taken user/email (#3093) * Correct error messages if user registers with taken user/email (fixes #2955) * exists --- crates/api_crud/src/user/create.rs | 24 +++++++----------------- crates/apub/src/objects/person.rs | 2 +- crates/db_schema/src/impls/local_user.rs | 12 ++++++++++-- crates/db_schema/src/impls/person.rs | 16 +++++++++++++--- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index d0aa05acc..f5a26f756 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -83,6 +83,12 @@ impl PerformCrud for Register { &context.settings().get_protocol_and_hostname(), )?; + if let Some(email) = &data.email { + if LocalUser::is_email_taken(context.pool(), email).await? { + return Err(LemmyError::from_message("email_already_exists")); + } + } + // We have to create both a person, and local_user // Register the new person @@ -116,23 +122,7 @@ impl PerformCrud for Register { .accepted_application(accepted_application) .build(); - let inserted_local_user = match LocalUser::create(context.pool(), &local_user_form).await { - Ok(lu) => lu, - Err(e) => { - let err_type = if e.to_string() - == "duplicate key value violates unique constraint \"local_user_email_key\"" - { - "email_already_exists" - } else { - "user_already_exists" - }; - - // If the local user creation errored, then delete that person - Person::delete(context.pool(), inserted_person.id).await?; - - return Err(LemmyError::from_error_message(e, err_type)); - } - }; + let inserted_local_user = LocalUser::create(context.pool(), &local_user_form).await?; if local_site.site_setup && require_registration_application { // Create the registration application diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index d3422b063..c71d46ccf 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -171,7 +171,7 @@ impl Object for ApubPerson { matrix_user_id: person.matrix_user_id, instance_id, }; - let person = DbPerson::create(context.pool(), &person_form).await?; + let person = DbPerson::upsert(context.pool(), &person_form).await?; Ok(person.into()) } diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 53e57ff85..c8ae23627 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -2,6 +2,7 @@ use crate::{ newtypes::LocalUserId, schema::local_user::dsl::{ accepted_application, + email, email_verified, local_user, password_encrypted, @@ -53,6 +54,14 @@ impl LocalUser { .get_results::(conn) .await } + + pub async fn is_email_taken(pool: &DbPool, email_: &str) -> Result { + use diesel::dsl::{exists, select}; + let conn = &mut get_conn(pool).await?; + select(exists(local_user.filter(email.eq(email_)))) + .get_result(conn) + .await + } } #[async_trait] @@ -80,8 +89,7 @@ impl Crud for LocalUser { let local_user_ = insert_into(local_user) .values(form_with_encrypted_password) .get_result::(conn) - .await - .expect("couldnt create local user"); + .await?; let site_languages = SiteLanguage::read_local_raw(pool).await; if let Ok(langs) = site_languages { diff --git a/crates/db_schema/src/impls/person.rs b/crates/db_schema/src/impls/person.rs index d2a7b08ce..5c23f8071 100644 --- a/crates/db_schema/src/impls/person.rs +++ b/crates/db_schema/src/impls/person.rs @@ -37,9 +37,6 @@ impl Crud for Person { let conn = &mut get_conn(pool).await?; insert_into(person::table) .values(form) - .on_conflict(person::actor_id) - .do_update() - .set(form) .get_result::(conn) .await } @@ -57,6 +54,19 @@ impl Crud for Person { } impl Person { + /// Update or insert the person. + /// + /// This is necessary for federation, because Activitypub doesnt distinguish between these actions. + pub async fn upsert(pool: &DbPool, form: &PersonInsertForm) -> Result { + let conn = &mut get_conn(pool).await?; + insert_into(person::table) + .values(form) + .on_conflict(person::actor_id) + .do_update() + .set(form) + .get_result::(conn) + .await + } pub async fn delete_account(pool: &DbPool, person_id: PersonId) -> Result { let conn = &mut get_conn(pool).await?;