Merge branches 'quenting/syn2mas/reader-writer-owned', 'quenting/syn2mas/ignore-things' and 'quenting/syn2mas/ignore-as-users' into quenting/syn2mas/merge-base

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)]
@@ -188,6 +193,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),
@@ -247,15 +266,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(
@@ -325,15 +345,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,
@@ -403,16 +424,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;
         }
 
@@ -428,7 +449,9 @@ async fn migrate_devices(
         // As we're using a real IP type in the MAS database, it is possible
         // that we encounter invalid IP addresses in the Synapse database.
         // In that case, we should ignore them, but still log a warning.
-        let last_active_ip = ip.and_then(|ip| {
+        // One special case: Synapse will record '-' as IP in some cases, we don't want
+        // to log about those
+        let last_active_ip = ip.filter(|ip| ip != "-").and_then(|ip| {
             ip.parse()
                 .map_err(|e| {
                     tracing::warn!(
@@ -497,16 +520,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;
         }
 
@@ -609,16 +632,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;
         }
 
@@ -715,15 +738,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

@@ -73,7 +73,7 @@ impl FullUserId {
             return Err(ExtractLocalpartError::NoAtSigil);
         };
 
-        let Some((localpart, server_name)) = without_sigil.split_once(':') else {
+        let Some((localpart, server_name)) = without_sigil.rsplit_once(':') else {
             return Err(ExtractLocalpartError::NoSeparator);
         };
 
@@ -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)
@@ -416,7 +417,7 @@ impl<'conn> SynapseReader<'conn> {
             SELECT
               user_id, device_id, display_name, last_seen, ip, user_agent
             FROM devices
-            WHERE NOT hidden
+            WHERE NOT hidden AND device_id != 'guest_device'
             ",
         )
         .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,
     },
 }