Fix problem where actors can have empty public key (fixes #2347) (#2348)

crates/api/src/lib.rs

@@ -230,6 +230,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "Gerry9812".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 

crates/api/src/local_user/save_settings.rs

@@ -87,7 +87,7 @@ impl Perform for SaveUserSettings {
     let default_listing_type = data.default_listing_type;
     let default_sort_type = data.default_sort_type;
     let password_encrypted = local_user_view.local_user.password_encrypted;
-    let public_key = local_user_view.person.public_key;
+    let public_key = Some(local_user_view.person.public_key);
 
     let person_form = PersonForm {
       name: local_user_view.person.name,

crates/api_crud/src/community/create.rs

@@ -93,7 +93,7 @@ impl PerformCrud for CreateCommunity {
       nsfw: data.nsfw,
       actor_id: Some(community_actor_id.to_owned()),
       private_key: Some(Some(keypair.private_key)),
-      public_key: keypair.public_key,
+      public_key: Some(keypair.public_key),
       followers_url: Some(generate_followers_url(&community_actor_id)?),
       inbox_url: Some(generate_inbox_url(&community_actor_id)?),
       shared_inbox_url: Some(Some(generate_shared_inbox_url(&community_actor_id)?)),

crates/api_crud/src/user/create.rs

@@ -118,7 +118,7 @@ impl PerformCrud for Register {
       name: data.username.to_owned(),
       actor_id: Some(actor_id.clone()),
       private_key: Some(Some(actor_keypair.private_key)),
-      public_key: actor_keypair.public_key,
+      public_key: Some(actor_keypair.public_key),
       inbox_url: Some(generate_inbox_url(&actor_id)?),
       shared_inbox_url: Some(Some(generate_shared_inbox_url(&actor_id)?)),
       admin: Some(no_admins),

crates/apub/src/collections/community_moderators.rs

@@ -169,6 +169,7 @@ mod tests {
 
     let old_mod = PersonForm {
       name: "holly".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
     let old_mod = Person::create(&context.pool().get().unwrap(), &old_mod).unwrap();

crates/apub/src/objects/person.rs

@@ -156,7 +156,7 @@ impl ApubObject for ApubPerson {
       admin: Some(false),
       bot_account: Some(person.kind == UserTypes::Service),
       private_key: None,
-      public_key: person.public_key.public_key_pem,
+      public_key: Some(person.public_key.public_key_pem),
       last_refreshed_at: Some(naive_now()),
       inbox_url: Some(person.inbox.into()),
       shared_inbox_url: Some(person.endpoints.map(|e| e.shared_inbox.into())),

crates/apub/src/protocol/objects/group.rs

@@ -88,7 +88,7 @@ impl Group {
       local: Some(false),
       private_key: None,
       hidden: Some(false),
-      public_key: self.public_key.public_key_pem,
+      public_key: Some(self.public_key.public_key_pem),
       last_refreshed_at: Some(naive_now()),
       icon: Some(self.icon.map(|i| i.url.into())),
       banner: Some(self.image.map(|i| i.url.into())),

crates/db_schema/src/aggregates/comment_aggregates.rs

@@ -35,6 +35,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "thommy_comment_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -42,6 +43,7 @@ mod tests {
 
     let another_person = PersonForm {
       name: "jerry_comment_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -50,6 +52,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "TIL_comment_agg".into(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/aggregates/community_aggregates.rs

@@ -35,6 +35,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "thommy_community_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -42,6 +43,7 @@ mod tests {
 
     let another_person = PersonForm {
       name: "jerry_community_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -50,6 +52,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "TIL_community_agg".into(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 
@@ -58,6 +61,7 @@ mod tests {
     let another_community = CommunityForm {
       name: "TIL_community_agg_2".into(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/aggregates/person_aggregates.rs

@@ -31,6 +31,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "thommy_user_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -38,6 +39,7 @@ mod tests {
 
     let another_person = PersonForm {
       name: "jerry_user_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -46,6 +48,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "TIL_site_agg".into(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/aggregates/post_aggregates.rs

@@ -31,6 +31,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "thommy_community_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -38,6 +39,7 @@ mod tests {
 
     let another_person = PersonForm {
       name: "jerry_community_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -46,6 +48,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "TIL_community_agg".into(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/aggregates/site_aggregates.rs

@@ -30,6 +30,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "thommy_site_agg".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -37,6 +38,7 @@ mod tests {
 
     let site_form = SiteForm {
       name: "test_site".into(),
+      public_key: Some("pubkey".to_string()),
       ..Default::default()
     };
 
@@ -45,6 +47,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "TIL_site_agg".into(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/impls/activity.rs

@@ -97,6 +97,7 @@ mod tests {
 
     let creator_form = PersonForm {
       name: "activity_creator_pm".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 

crates/db_schema/src/impls/comment.rs

@@ -227,6 +227,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "terry".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -235,6 +236,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "test community".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/impls/community.rs

@@ -377,6 +377,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "bobbee".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -385,7 +386,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "TIL".into(),
       title: "nada".to_owned(),
-      public_key: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 
@@ -404,7 +405,7 @@ mod tests {
       actor_id: inserted_community.actor_id.to_owned(),
       local: true,
       private_key: None,
-      public_key: "nada".to_owned(),
+      public_key: "pubkey".to_owned(),
       last_refreshed_at: inserted_community.published,
       icon: None,
       banner: None,

crates/db_schema/src/impls/moderator.rs

@@ -372,6 +372,7 @@ mod tests {
 
     let new_mod = PersonForm {
       name: "the mod".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -379,6 +380,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "jim2".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -387,6 +389,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "mod_community".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/impls/password_reset_request.rs

@@ -87,6 +87,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "thommy prw".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 

crates/db_schema/src/impls/person.rs

@@ -343,7 +343,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "holly".into(),
-      public_key: "nada".to_owned(),
+      public_key: Some("nada".to_owned()),
       ..PersonForm::default()
     };
 

crates/db_schema/src/impls/person_mention.rs

@@ -97,6 +97,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "terrylake".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -104,6 +105,7 @@ mod tests {
 
     let recipient_form = PersonForm {
       name: "terrylakes recipient".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -112,6 +114,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "test community lake".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/impls/post.rs

@@ -343,6 +343,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "jim".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -351,6 +352,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "test community_3".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_schema/src/impls/private_message.rs

@@ -151,6 +151,7 @@ mod tests {
 
     let creator_form = PersonForm {
       name: "creator_pm".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -158,6 +159,7 @@ mod tests {
 
     let recipient_form = PersonForm {
       name: "recipient_pm".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 

crates/db_schema/src/source/community.rs

@@ -68,7 +68,7 @@ pub struct CommunityForm {
   pub actor_id: Option<DbUrl>,
   pub local: Option<bool>,
   pub private_key: Option<Option<String>>,
-  pub public_key: String,
+  pub public_key: Option<String>,
   pub last_refreshed_at: Option<chrono::NaiveDateTime>,
   pub icon: Option<Option<DbUrl>>,
   pub banner: Option<Option<DbUrl>>,

crates/db_schema/src/source/person.rs

@@ -172,7 +172,7 @@ pub struct PersonForm {
   pub bio: Option<Option<String>>,
   pub local: Option<bool>,
   pub private_key: Option<Option<String>>,
-  pub public_key: String,
+  pub public_key: Option<String>,
   pub last_refreshed_at: Option<chrono::NaiveDateTime>,
   pub banner: Option<Option<DbUrl>>,
   pub deleted: Option<bool>,

crates/db_views/src/comment_report_view.rs

@@ -321,6 +321,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "timmy_crv".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -328,6 +329,7 @@ mod tests {
 
     let new_person_2 = PersonForm {
       name: "sara_crv".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -336,6 +338,7 @@ mod tests {
     // Add a third person, since new ppl can only report something once.
     let new_person_3 = PersonForm {
       name: "jessica_crv".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -344,6 +347,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "test community crv".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_views/src/comment_view.rs

@@ -551,6 +551,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "timmy".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -558,6 +559,7 @@ mod tests {
 
     let new_person_2 = PersonForm {
       name: "sara".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -566,6 +568,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "test community 5".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_views/src/post_report_view.rs

@@ -310,6 +310,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: "timmy_prv".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -317,6 +318,7 @@ mod tests {
 
     let new_person_2 = PersonForm {
       name: "sara_prv".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -325,6 +327,7 @@ mod tests {
     // Add a third person, since new ppl can only report something once.
     let new_person_3 = PersonForm {
       name: "jessica_prv".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -333,6 +336,7 @@ mod tests {
     let new_community = CommunityForm {
       name: "test community prv".to_string(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 

crates/db_views/src/post_view.rs

@@ -535,6 +535,7 @@ mod tests {
 
     let new_person = PersonForm {
       name: person_name.to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -543,6 +544,7 @@ mod tests {
     let new_bot = PersonForm {
       name: person_name.to_owned(),
       bot_account: Some(true),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -551,6 +553,7 @@ mod tests {
     let new_community = CommunityForm {
       name: community_name.to_owned(),
       title: "nada".to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..CommunityForm::default()
     };
 
@@ -559,6 +562,7 @@ mod tests {
     // Test a person block, make sure the post query doesn't include their post
     let blocked_person = PersonForm {
       name: person_name.to_owned(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 

crates/db_views/src/registration_application_view.rs

@@ -187,6 +187,7 @@ mod tests {
     let timmy_person_form = PersonForm {
       name: "timmy_rav".into(),
       admin: Some(true),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -202,6 +203,7 @@ mod tests {
 
     let sara_person_form = PersonForm {
       name: "sara_rav".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 
@@ -228,6 +230,7 @@ mod tests {
 
     let jess_person_form = PersonForm {
       name: "jess_rav".into(),
+      public_key: Some("pubkey".to_string()),
       ..PersonForm::default()
     };
 

src/code_migrations.rs

@@ -25,6 +25,7 @@ use lemmy_db_schema::{
   utils::naive_now,
 };
 use lemmy_utils::error::LemmyError;
+use std::default::Default;
 use tracing::info;
 use url::Url;
 
@@ -40,6 +41,7 @@ pub fn run_advanced_migrations(
   post_thumbnail_url_updates_2020_07_27(conn, protocol_and_hostname)?;
   apub_columns_2021_02_02(conn)?;
   instance_actor_2022_01_28(conn, protocol_and_hostname)?;
+  regenerate_public_keys_2022_07_05(conn)?;
 
   Ok(())
 }
@@ -69,7 +71,7 @@ fn user_updates_2020_04_02(
         protocol_and_hostname,
       )?),
       private_key: Some(Some(keypair.private_key)),
-      public_key: keypair.public_key,
+      public_key: Some(keypair.public_key),
       last_refreshed_at: Some(naive_now()),
       ..PersonForm::default()
     };
@@ -112,7 +114,7 @@ fn community_updates_2020_04_02(
       actor_id: Some(community_actor_id.to_owned()),
       local: Some(ccommunity.local),
       private_key: Some(Some(keypair.private_key)),
-      public_key: keypair.public_key,
+      public_key: Some(keypair.public_key),
       last_refreshed_at: Some(naive_now()),
       icon: Some(ccommunity.icon.to_owned()),
       banner: Some(ccommunity.banner.to_owned()),
@@ -294,6 +296,10 @@ fn instance_actor_2022_01_28(
 ) -> Result<(), LemmyError> {
   info!("Running instance_actor_2021_09_29");
   if let Ok(site) = Site::read_local_site(conn) {
+    // if site already has public key, we dont need to do anything here
+    if !site.public_key.is_empty() {
+      return Ok(());
+    }
     let key_pair = generate_actor_keypair()?;
     let actor_id = Url::parse(protocol_and_hostname)?;
     let site_form = SiteForm {
@@ -309,3 +315,60 @@ fn instance_actor_2022_01_28(
   }
   Ok(())
 }
+
+/// Fix for bug #2347, which can result in community/person public keys being overwritten with
+/// empty string when the database value is updated. We go through all actors, and if the public
+/// key field is empty, generate a new keypair. It would be possible to regenerate only the pubkey,
+/// but thats more complicated and has no benefit, as federation is already broken for these actors.
+/// https://github.com/LemmyNet/lemmy/issues/2347
+fn regenerate_public_keys_2022_07_05(conn: &PgConnection) -> Result<(), LemmyError> {
+  info!("Running regenerate_public_keys_2022_07_05");
+
+  {
+    // update communities with empty pubkey
+    use lemmy_db_schema::schema::community::dsl::*;
+    let communities: Vec<Community> = community
+      .filter(local.eq(true))
+      .filter(public_key.eq(""))
+      .load::<Community>(conn)?;
+    for community_ in communities {
+      info!(
+        "local community {} has empty public key field, regenerating key",
+        community_.name
+      );
+      let key_pair = generate_actor_keypair()?;
+      let form = CommunityForm {
+        name: community_.name,
+        title: community_.title,
+        public_key: Some(key_pair.public_key),
+        private_key: Some(Some(key_pair.private_key)),
+        ..Default::default()
+      };
+      Community::update(conn, community_.id, &form)?;
+    }
+  }
+
+  {
+    // update persons with empty pubkey
+    use lemmy_db_schema::schema::person::dsl::*;
+    let persons = person
+      .filter(local.eq(true))
+      .filter(public_key.eq(""))
+      .load::<Person>(conn)?;
+    for person_ in persons {
+      info!(
+        "local user {} has empty public key field, regenerating key",
+        person_.name
+      );
+      let key_pair = generate_actor_keypair()?;
+      let form = PersonForm {
+        name: person_.name,
+        public_key: Some(key_pair.public_key),
+        private_key: Some(Some(key_pair.private_key)),
+        ..Default::default()
+      };
+      Person::update(conn, person_.id, &form)?;
+    }
+  }
+  Ok(())
+}