Adding fixes from comments.

crates/api/src/local_user.rs

@@ -183,32 +183,30 @@ impl Perform for SaveUserSettings {
     let matrix_user_id = diesel_option_overwrite(&data.matrix_user_id);
     let bot_account = data.bot_account;
     let email_deref = data.email.as_deref().map(|e| e.to_owned());
+    let email = diesel_option_overwrite(&email_deref);
 
-    let email = if let Some(email) = &email_deref {
+    if let Some(Some(email)) = &email {
-      let site = blocking(context.pool(), Site::read_simple).await??;
+      let previous_email = local_user_view.local_user.email.unwrap_or_default();
-      if site.require_email_verification {
+      // Only send the verification email if there was an email change
-        if let Some(previous_email) = local_user_view.local_user.email {
+      if previous_email.ne(email) {
-          // Only send the verification email if there was an email change
+        send_verification_email(
-          if previous_email.ne(email) {
+          local_user_view.local_user.id,
-            send_verification_email(
+          email,
-              local_user_view.local_user.id,
+          &local_user_view.person.name,
-              email,
+          context.pool(),
-              &local_user_view.person.name,
+          &context.settings(),
-              context.pool(),
+        )
-              &context.settings(),
+        .await?;
-            )
-            .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)
       }
-    } else {
+    }
-      None
+
-    };
+    // When the site requires email, make sure email is not Some(None). IE, an overwrite to a None value
+    if let Some(email) = &email {
+      let site_fut = blocking(context.pool(), Site::read_simple);
+      if email.is_none() && site_fut.await??.require_email_verification {
+        return Err(LemmyError::from_message("email_required"));
+      }
+    }
 
     if let Some(Some(bio)) = &bio {
       if bio.chars().count() > 300 {
@@ -924,7 +922,7 @@ impl Perform for GetUnreadCount {
 
 #[async_trait::async_trait(?Send)]
 impl Perform for VerifyEmail {
-  type Response = LoginResponse;
+  type Response = VerifyEmailResponse;
 
   async fn perform(
     &self,
@@ -964,21 +962,6 @@ impl Perform for VerifyEmail {
     })
     .await??;
 
-    let site = blocking(context.pool(), Site::read_simple).await??;
+    Ok(VerifyEmailResponse {})
-    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

@@ -658,16 +658,15 @@ impl Perform for ApproveRegistrationApplication {
     })
     .await??;
 
-    let require_email_verification = blocking(context.pool(), Site::read_simple)
+    if data.approve {
-      .await??
-      .require_email_verification;
-
-    if require_email_verification && data.approve {
       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())?;
+
+      if approved_local_user_view.local_user.email.is_some() {
+        send_application_approved_email(&approved_local_user_view, &context.settings())?;
+      }
     }
 
     // Read the view

crates/api_common/src/lib.rs

@@ -468,7 +468,10 @@ pub async fn check_registration_application(
   local_user_view: &LocalUserView,
   pool: &DbPool,
 ) -> Result<(), LemmyError> {
-  if site.require_application && !local_user_view.local_user.accepted_application {
+  if site.require_application
+    && !local_user_view.local_user.accepted_application
+    && !local_user_view.person.admin
+  {
     // Fetch the registration, see if its denied
     let local_user_id = local_user_view.local_user.id;
     let registration = blocking(pool, move |conn| {
@@ -483,3 +486,20 @@ pub async fn check_registration_application(
   }
   Ok(())
 }
+
+/// TODO this check should be removed after https://github.com/LemmyNet/lemmy/issues/868 is done.
+pub async fn check_private_instance_and_federation_enabled(
+  pool: &DbPool,
+  settings: &Settings,
+) -> Result<(), LemmyError> {
+  let site_opt = blocking(pool, Site::read_simple).await?;
+
+  if let Ok(site) = site_opt {
+    if site.private_instance && settings.federation.enabled {
+      return Err(LemmyError::from_message(
+        "Cannot have both private instance and federation enabled.",
+      ));
+    }
+  }
+  Ok(())
+}

crates/api_common/src/person.rs

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

crates/api_crud/src/site/update.rs

@@ -11,7 +11,10 @@ use lemmy_db_schema::{
   diesel_option_overwrite,
   diesel_option_overwrite_to_url,
   naive_now,
-  source::site::{Site, SiteForm},
+  source::{
+    local_user::LocalUser,
+    site::{Site, SiteForm},
+  },
   traits::Crud,
 };
 use lemmy_db_views::site_view::SiteView;
@@ -68,11 +71,35 @@ impl PerformCrud for EditSite {
       private_instance: data.private_instance,
     };
 
-    let update_site = move |conn: &'_ _| Site::update(conn, 1, &site_form);
+    let update_site = blocking(context.pool(), move |conn| {
-    blocking(context.pool(), update_site)
+      Site::update(conn, 1, &site_form)
+    })
+    .await?
+    .map_err(LemmyError::from)
+    .map_err(|e| e.with_message("couldnt_update_site"))?;
+
+    // TODO can't think of a better way to do this.
+    // If the server suddenly requires email verification, or required applications, no old users
+    // will be able to log in. It really only wants this to be a requirement for NEW signups.
+    // So if it was set from false, to true, you need to update all current users columns to be verified.
+
+    if !found_site.require_application && update_site.require_application {
+      blocking(context.pool(), move |conn| {
+        LocalUser::set_all_users_registration_applications_accepted(conn)
+      })
       .await?
       .map_err(LemmyError::from)
-      .map_err(|e| e.with_message("couldnt_update_site"))?;
+      .map_err(|e| e.with_message("couldnt_set_all_registrations_accepted"))?;
+    }
+
+    if !found_site.require_email_verification && update_site.require_email_verification {
+      blocking(context.pool(), move |conn| {
+        LocalUser::set_all_users_email_verified(conn)
+      })
+      .await?
+      .map_err(LemmyError::from)
+      .map_err(|e| e.with_message("couldnt_set_all_email_verified"))?;
+    }
 
     let site_view = blocking(context.pool(), SiteView::read).await??;
 

crates/db_schema/src/impls/local_user.rs

@@ -89,6 +89,20 @@ impl LocalUser {
       ))
       .get_result::<Self>(conn)
   }
+
+  pub fn set_all_users_email_verified(conn: &PgConnection) -> Result<Vec<Self>, Error> {
+    diesel::update(local_user)
+      .set(email_verified.eq(true))
+      .get_results::<Self>(conn)
+  }
+
+  pub fn set_all_users_registration_applications_accepted(
+    conn: &PgConnection,
+  ) -> Result<Vec<Self>, Error> {
+    diesel::update(local_user)
+      .set(accepted_application.eq(true))
+      .get_results::<Self>(conn)
+  }
 }
 
 impl Crud for LocalUser {

src/main.rs

@@ -9,7 +9,7 @@ use diesel::{
 };
 use doku::json::{AutoComments, Formatting};
 use lemmy_api::match_websocket_operation;
-use lemmy_api_common::blocking;
+use lemmy_api_common::{blocking, check_private_instance_and_federation_enabled};
 use lemmy_api_crud::match_websocket_operation_crud;
 use lemmy_apub_lib::activity_queue::create_activity_queue;
 use lemmy_db_schema::{get_database_url_from_env, source::secret::Secret};
@@ -103,6 +103,8 @@ async fn main() -> Result<(), LemmyError> {
 
   let activity_queue = queue_manager.queue_handle().clone();
 
+  check_private_instance_and_federation_enabled(&pool, &settings).await?;
+
   let chat_server = ChatServer::startup(
     pool.clone(),
     rate_limiter.clone(),