A few fixes for private sites:

- Added a check_registration_application function.
- Only send a verification email if its been changed.
- VerifyEmail now returns LoginResponse.
- Deleting the old tokens after a successful email verify.
- If port is missing on email config, display a better error message.

crates/api/src/local_user.rs

@@ -6,6 +6,7 @@ use captcha::{gen, Difficulty};
 use chrono::Duration;
 use lemmy_api_common::{
   blocking,
+  check_registration_application,
   get_local_user_view_from_jwt,
   is_admin,
   password_length_check,
@@ -97,9 +98,7 @@ impl Perform for Login {
       return Err(LemmyError::from_message("email_not_verified"));
     }
 
-    if site.require_application && !local_user_view.local_user.accepted_application {
-      return Err(LemmyError::from_message("registration_application_pending"));
-    }
+    check_registration_application(&site, &local_user_view, context.pool()).await?;
 
     // Return the jwt
     Ok(LoginResponse {
@@ -188,14 +187,21 @@ impl Perform for SaveUserSettings {
     let email = if let Some(email) = &email_deref {
       let site = blocking(context.pool(), Site::read_simple).await??;
       if site.require_email_verification {
-        send_verification_email(
-          local_user_view.local_user.id,
-          email,
-          &local_user_view.person.name,
-          context.pool(),
-          &context.settings(),
-        )
-        .await?;
+        if let Some(previous_email) = local_user_view.local_user.email {
+          // Only send the verification email if there was an email change
+          if previous_email.ne(email) {
+            send_verification_email(
+              local_user_view.local_user.id,
+              email,
+              &local_user_view.person.name,
+              context.pool(),
+              &context.settings(),
+            )
+            .await?;
+          }
+        }
+        // Fine to return None here, because the actual email is also in the email_verification
+        // table, and gets set in the function below.
         None
       } else {
         diesel_option_overwrite(&email_deref)
@@ -918,7 +924,7 @@ impl Perform for GetUnreadCount {
 
 #[async_trait::async_trait(?Send)]
 impl Perform for VerifyEmail {
-  type Response = VerifyEmailResponse;
+  type Response = LoginResponse;
 
   async fn perform(
     &self,
@@ -953,6 +959,26 @@ impl Perform for VerifyEmail {
 
     send_email_verification_success(&local_user_view, &context.settings())?;
 
-    Ok(VerifyEmailResponse {})
+    blocking(context.pool(), move |conn| {
+      EmailVerification::delete_old_tokens_for_local_user(conn, local_user_id)
+    })
+    .await??;
+
+    let site = blocking(context.pool(), Site::read_simple).await??;
+    check_registration_application(&site, &local_user_view, context.pool()).await?;
+
+    // Return the jwt
+    Ok(LoginResponse {
+      jwt: Some(
+        Claims::jwt(
+          local_user_view.local_user.id.0,
+          &context.secret().jwt_secret,
+          &context.settings().hostname,
+        )?
+        .into(),
+      ),
+      verify_email_sent: false,
+      registration_created: false,
+    })
   }
 }

crates/api/src/site.rs

@@ -38,6 +38,7 @@ use lemmy_db_schema::{
 };
 use lemmy_db_views::{
   comment_view::{CommentQueryBuilder, CommentView},
+  local_user_view::LocalUserView,
   post_view::{PostQueryBuilder, PostView},
   registration_application_view::{
     RegistrationApplicationQueryBuilder,
@@ -662,7 +663,11 @@ impl Perform for ApproveRegistrationApplication {
       .require_email_verification;
 
     if require_email_verification && data.approve {
-      send_application_approved_email(&local_user_view, &context.settings())?;
+      let approved_local_user_view = blocking(context.pool(), move |conn| {
+        LocalUserView::read(conn, approved_user_id)
+      })
+      .await??;
+      send_application_approved_email(&approved_local_user_view, &context.settings())?;
     }
 
     // Read the view

crates/api_common/src/lib.rs

@@ -14,6 +14,7 @@ use lemmy_db_schema::{
     password_reset_request::PasswordResetRequest,
     person_block::PersonBlock,
     post::{Post, PostRead, PostReadForm},
+    registration_application::RegistrationApplication,
     secret::Secret,
     site::Site,
   },
@@ -426,8 +427,8 @@ pub async fn send_verification_email(
   let body = format!(
     concat!(
       "Please click the link below to verify your email address ",
-      "for the account @{}@{}. Ignore this email if the account isn't yours.\n\n",
-      "<a href=\"{}\"></a>"
+      "for the account @{}@{}. Ignore this email if the account isn't yours.<br><br>",
+      "<a href=\"{}\">Verify your email</a>"
     ),
     username, settings.hostname, verify_link
   );
@@ -441,7 +442,7 @@ pub fn send_email_verification_success(
   settings: &Settings,
 ) -> Result<(), LemmyError> {
   let email = &local_user_view.local_user.email.to_owned().expect("email");
-  let subject = &format!("Email verified for {}", local_user_view.person.name);
+  let subject = &format!("Email verified for {}", local_user_view.person.actor_id);
   let html = "Your email has been verified.";
   send_email(subject, email, &local_user_view.person.name, html, settings)
 }
@@ -452,8 +453,8 @@ pub fn send_application_approved_email(
 ) -> Result<(), LemmyError> {
   let email = &local_user_view.local_user.email.to_owned().expect("email");
   let subject = &format!(
-    "Registration to {} approved for {}",
-    settings.hostname, local_user_view.person.name
+    "Registration approved for {}",
+    local_user_view.person.actor_id
   );
   let html = &format!(
     "Your registration application has been approved. Welcome to {}!",
@@ -461,3 +462,24 @@ pub fn send_application_approved_email(
   );
   send_email(subject, email, &local_user_view.person.name, html, settings)
 }
+
+pub async fn check_registration_application(
+  site: &Site,
+  local_user_view: &LocalUserView,
+  pool: &DbPool,
+) -> Result<(), LemmyError> {
+  if site.require_application && !local_user_view.local_user.accepted_application {
+    // Fetch the registration, see if its denied
+    let local_user_id = local_user_view.local_user.id;
+    let registration = blocking(pool, move |conn| {
+      RegistrationApplication::find_by_local_user_id(conn, local_user_id)
+    })
+    .await??;
+    if registration.deny_reason.is_some() {
+      return Err(LemmyError::from_message("registration_denied"));
+    } else {
+      return Err(LemmyError::from_message("registration_application_pending"));
+    }
+  }
+  Ok(())
+}

crates/api_common/src/person.rs

@@ -293,6 +293,3 @@ pub struct GetUnreadCountResponse {
 pub struct VerifyEmail {
   pub token: String,
 }
-
-#[derive(Debug, Serialize, Deserialize, Clone)]
-pub struct VerifyEmailResponse {}

crates/api_crud/src/site/read.rs

@@ -147,12 +147,6 @@ impl PerformCrud for GetSite {
         person_blocks,
       })
     } else {
-      // If the site is setup, private, and there is no auth, return an error
-      if let Some(site_view) = site_view.to_owned() {
-        if site_view.site.private_instance {
-          return Err(LemmyError::from_message("instance_is_private"));
-        }
-      }
       None
     };
 

crates/api_crud/src/user/create.rs

@@ -257,43 +257,41 @@ impl PerformCrud for Register {
         .map_err(|e| e.with_message("community_moderator_already_exists"))?;
     }
 
-    // Log the user in directly if email verification is not required
-    let login_response = if email_verification {
-      send_verification_email(
-        inserted_local_user.id,
-        // we check at the beginning of this method that email is set
-        &inserted_local_user.email.expect("email was provided"),
-        &inserted_person.name,
-        context.pool(),
-        &context.settings(),
-      )
-      .await?;
-      LoginResponse {
-        jwt: None,
-        verify_email_sent: true,
-        registration_created: false,
-      }
-    } else if require_application {
-      LoginResponse {
-        jwt: None,
-        verify_email_sent: false,
-        registration_created: true,
-      }
-    } else {
-      LoginResponse {
-        jwt: Some(
-          Claims::jwt(
-            inserted_local_user.id.0,
-            &context.secret().jwt_secret,
-            &context.settings().hostname,
-          )?
-          .into(),
-        ),
-        verify_email_sent: false,
-        registration_created: false,
-      }
+    let mut login_response = LoginResponse {
+      jwt: None,
+      registration_created: false,
+      verify_email_sent: false,
     };
 
+    // Log the user in directly if email verification and application aren't required
+    if !require_application && !email_verification {
+      login_response.jwt = Some(
+        Claims::jwt(
+          inserted_local_user.id.0,
+          &context.secret().jwt_secret,
+          &context.settings().hostname,
+        )?
+        .into(),
+      );
+    } else {
+      if email_verification {
+        send_verification_email(
+          inserted_local_user.id,
+          // we check at the beginning of this method that email is set
+          &inserted_local_user.email.expect("email was provided"),
+          &inserted_person.name,
+          context.pool(),
+          &context.settings(),
+        )
+        .await?;
+        login_response.verify_email_sent = true;
+      }
+
+      if require_application {
+        login_response.registration_created = true;
+      }
+    }
+
     Ok(login_response)
   }
 }

crates/db_schema/src/impls/email_verification.rs

@@ -1,4 +1,4 @@
-use crate::{source::email_verification::*, traits::Crud};
+use crate::{newtypes::LocalUserId, source::email_verification::*, traits::Crud};
 use diesel::{insert_into, result::Error, ExpressionMethods, PgConnection, QueryDsl, RunQueryDsl};
 
 impl Crud for EmailVerification {
@@ -36,4 +36,11 @@ impl EmailVerification {
       .filter(verification_token.eq(token))
       .first::<Self>(conn)
   }
+  pub fn delete_old_tokens_for_local_user(
+    conn: &PgConnection,
+    local_user_id_: LocalUserId,
+  ) -> Result<usize, Error> {
+    use crate::schema::email_verification::dsl::*;
+    diesel::delete(email_verification.filter(local_user_id.eq(local_user_id_))).execute(conn)
+  }
 }

crates/db_schema/src/impls/registration_application.rs

@@ -1,5 +1,5 @@
-use crate::{source::registration_application::*, traits::Crud};
-use diesel::{insert_into, result::Error, PgConnection, QueryDsl, RunQueryDsl};
+use crate::{newtypes::LocalUserId, source::registration_application::*, traits::Crud};
+use diesel::{insert_into, result::Error, ExpressionMethods, PgConnection, QueryDsl, RunQueryDsl};
 
 impl Crud for RegistrationApplication {
   type Form = RegistrationApplicationForm;
@@ -28,3 +28,15 @@ impl Crud for RegistrationApplication {
     diesel::delete(registration_application.find(id_)).execute(conn)
   }
 }
+
+impl RegistrationApplication {
+  pub fn find_by_local_user_id(
+    conn: &PgConnection,
+    local_user_id_: LocalUserId,
+  ) -> Result<Self, Error> {
+    use crate::schema::registration_application::dsl::*;
+    registration_application
+      .filter(local_user_id.eq(local_user_id_))
+      .first::<Self>(conn)
+  }
+}

crates/db_schema/src/newtypes.rs

@@ -43,7 +43,9 @@ impl fmt::Display for CommentId {
 )]
 pub struct CommunityId(pub i32);
 
-#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, Serialize, Deserialize, DieselNewType)]
+#[derive(
+  Debug, Copy, Clone, Hash, Eq, PartialEq, Default, Serialize, Deserialize, DieselNewType,
+)]
 pub struct LocalUserId(pub i32);
 
 #[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, Serialize, Deserialize, DieselNewType)]

crates/utils/src/email.rs

@@ -29,6 +29,12 @@ pub fn send_email(
 
   let (smtp_server, smtp_port) = {
     let email_and_port = email_config.smtp_server.split(':').collect::<Vec<&str>>();
+    if email_and_port.len() == 1 {
+      return Err(LemmyError::from_message(
+        "email.smtp_server needs a port, IE smtp.xxx.com:465",
+      ));
+    }
+
     (
       email_and_port[0],
       email_and_port[1]

migrations/2021-11-23-132840_email_verification/up.sql

@@ -11,5 +11,4 @@ create table email_verification (
     local_user_id int references local_user(id) on update cascade on delete cascade not null,
     email text not null,
     verification_token text not null
-
-);
+);