Actualy read appservice users, but don't insert them

Author: sandhose

crates/syn2mas/src/migration.rs

@@ -74,6 +74,7 @@ bitflags::bitflags! {
         const IS_SYNAPSE_ADMIN = 0b0000_0001;
         const IS_DEACTIVATED = 0b0000_0010;
         const IS_GUEST = 0b0000_0100;
+        const IS_APPSERVICE = 0b0000_1000;
     }
 }
 
@@ -89,6 +90,10 @@ impl UserFlags {
     const fn is_synapse_admin(self) -> bool {
         self.contains(UserFlags::IS_SYNAPSE_ADMIN)
     }
+
+    const fn is_appservice(self) -> bool {
+        self.contains(UserFlags::IS_APPSERVICE)
+    }
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -177,6 +182,20 @@ async fn migrate_users(
         if bool::from(user.is_guest) {
             flags |= UserFlags::IS_GUEST;
         }
+        if user.appservice_id.is_some() {
+            flags |= UserFlags::IS_APPSERVICE;
+
+            // Special case for appservice users: we don't insert them into the database
+            // We just record the user's information in the state and continue
+            state.users.insert(
+                CompactString::new(&mas_user.username),
+                UserInfo {
+                    mas_user_id: Uuid::nil(),
+                    flags,
+                },
+            );
+            continue;
+        }
 
         state.users.insert(
             CompactString::new(&mas_user.username),
@@ -233,15 +252,16 @@ async fn migrate_threepids(
             .into_extract_localpart(synapse_user_id.clone())?
             .to_owned();
         let Some(user_infos) = state.users.get(username.as_str()).copied() else {
-            if is_likely_appservice(&username) {
-                continue;
-            }
             return Err(Error::MissingUserFromDependentTable {
                 table: "user_threepids".to_owned(),
                 user: synapse_user_id,
             });
         };
 
+        if user_infos.flags.is_appservice() {
+            continue;
+        }
+
         if medium == "email" {
             email_buffer
                 .write(
@@ -311,15 +331,16 @@ async fn migrate_external_ids(
             .into_extract_localpart(synapse_user_id.clone())?
             .to_owned();
         let Some(user_infos) = state.users.get(username.as_str()).copied() else {
-            if is_likely_appservice(&username) {
-                continue;
-            }
             return Err(Error::MissingUserFromDependentTable {
                 table: "user_external_ids".to_owned(),
                 user: synapse_user_id,
             });
         };
 
+        if user_infos.flags.is_appservice() {
+            continue;
+        }
+
         let Some(&upstream_provider_id) = state.provider_id_mapping.get(&auth_provider) else {
             return Err(Error::MissingAuthProviderMapping {
                 synapse_id: auth_provider,
@@ -389,16 +410,16 @@ async fn migrate_devices(
             .into_extract_localpart(synapse_user_id.clone())?
             .to_owned();
         let Some(user_infos) = state.users.get(username.as_str()).copied() else {
-            if is_likely_appservice(&username) {
-                continue;
-            }
             return Err(Error::MissingUserFromDependentTable {
                 table: "devices".to_owned(),
                 user: synapse_user_id,
             });
         };
 
-        if user_infos.flags.is_deactivated() || user_infos.flags.is_guest() {
+        if user_infos.flags.is_deactivated()
+            || user_infos.flags.is_guest()
+            || user_infos.flags.is_appservice()
+        {
             continue;
         }
 
@@ -483,16 +504,16 @@ async fn migrate_unrefreshable_access_tokens(
             .into_extract_localpart(synapse_user_id.clone())?
             .to_owned();
         let Some(user_infos) = state.users.get(username.as_str()).copied() else {
-            if is_likely_appservice(&username) {
-                continue;
-            }
             return Err(Error::MissingUserFromDependentTable {
                 table: "access_tokens".to_owned(),
                 user: synapse_user_id,
             });
         };
 
-        if user_infos.flags.is_deactivated() || user_infos.flags.is_guest() {
+        if user_infos.flags.is_deactivated()
+            || user_infos.flags.is_guest()
+            || user_infos.flags.is_appservice()
+        {
             continue;
         }
 
@@ -595,16 +616,16 @@ async fn migrate_refreshable_token_pairs(
             .into_extract_localpart(synapse_user_id.clone())?
             .to_owned();
         let Some(user_infos) = state.users.get(username.as_str()).copied() else {
-            if is_likely_appservice(&username) {
-                continue;
-            }
             return Err(Error::MissingUserFromDependentTable {
                 table: "refresh_tokens".to_owned(),
                 user: synapse_user_id,
             });
         };
 
-        if user_infos.flags.is_deactivated() || user_infos.flags.is_guest() {
+        if user_infos.flags.is_deactivated()
+            || user_infos.flags.is_guest()
+            || user_infos.flags.is_appservice()
+        {
             continue;
         }
 
@@ -701,15 +722,3 @@ fn transform_user(
 
     Ok((new_user, mas_password))
 }
-
-/// Returns true if and only if the given localpart looks like it would belong
-/// to an application service user.
-/// The rule here is that it must start with an underscore.
-/// Synapse reserves these by default, but there is no hard rule prohibiting
-/// other namespaces from being reserved, so this is not a robust check.
-// TODO replace with a more robust mechanism, if we even care about this sanity check
-// e.g. read application service registration files.
-#[inline]
-fn is_likely_appservice(localpart: &str) -> bool {
-    localpart.starts_with('_')
-}

crates/syn2mas/src/synapse_reader/mod.rs

@@ -192,6 +192,8 @@ pub struct SynapseUser {
     /// account!
     pub is_guest: SynapseBool,
     // TODO do we care about upgrade_ts (users who upgraded from guest accounts to real accounts)
+    /// The ID of the appservice that created this user, if any.
+    pub appservice_id: Option<String>,
 }
 
 /// Row of the `user_threepids` table in Synapse.
@@ -369,9 +371,8 @@ impl<'conn> SynapseReader<'conn> {
         sqlx::query_as(
             "
             SELECT
-              name, password_hash, admin, deactivated, creation_ts, is_guest
+              name, password_hash, admin, deactivated, creation_ts, is_guest, appservice_id
             FROM users
-            WHERE appservice_id IS NULL
             ",
         )
         .fetch(&mut *self.txn)

crates/syn2mas/src/synapse_reader/snapshots/syn2mas__synapse_reader__test__read_users.snap

@@ -22,5 +22,6 @@ expression: users
         is_guest: SynapseBool(
             false,
         ),
+        appservice_id: None,
     },
 }