Skip to content

Commit af99243

Browse files
committed
Actualy read appservice users, but don't insert them
1 parent af27db6 commit af99243

File tree

3 files changed

+44
-51
lines changed

3 files changed

+44
-51
lines changed

crates/syn2mas/src/migration.rs

Lines changed: 40 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ bitflags::bitflags! {
8484
const IS_SYNAPSE_ADMIN = 0b0000_0001;
8585
const IS_DEACTIVATED = 0b0000_0010;
8686
const IS_GUEST = 0b0000_0100;
87+
const IS_APPSERVICE = 0b0000_1000;
8788
}
8889
}
8990

@@ -99,6 +100,10 @@ impl UserFlags {
99100
const fn is_synapse_admin(self) -> bool {
100101
self.contains(UserFlags::IS_SYNAPSE_ADMIN)
101102
}
103+
104+
const fn is_appservice(self) -> bool {
105+
self.contains(UserFlags::IS_APPSERVICE)
106+
}
102107
}
103108

104109
#[derive(Debug, Clone, Copy)]
@@ -254,6 +259,21 @@ async fn migrate_users(
254259
flags |= UserFlags::IS_GUEST;
255260
}
256261

262+
if user.appservice_id.is_some() {
263+
flags |= UserFlags::IS_APPSERVICE;
264+
265+
// Special case for appservice users: we don't insert them into the database
266+
// We just record the user's information in the state and continue
267+
state.users.insert(
268+
CompactString::new(&mas_user.username),
269+
UserInfo {
270+
mas_user_id: Uuid::nil(),
271+
flags,
272+
},
273+
);
274+
continue;
275+
}
276+
257277
state.users.insert(
258278
CompactString::new(&mas_user.username),
259279
UserInfo {
@@ -349,10 +369,6 @@ async fn migrate_threepids(
349369
continue;
350370
};
351371
let Some(user_infos) = state.users.get(username.as_str()).copied() else {
352-
if is_likely_appservice(&username) {
353-
continue;
354-
}
355-
356372
// HACK(matrix.org): we seem to have many threepids for unknown users
357373
if state.users.contains_key(username.to_lowercase().as_str()) {
358374
tracing::warn!(mxid = %synapse_user_id, "Threepid found in the database matching an MXID with the wrong casing");
@@ -365,6 +381,10 @@ async fn migrate_threepids(
365381
});
366382
};
367383

384+
if user_infos.flags.is_appservice() {
385+
continue;
386+
}
387+
368388
if medium == "email" {
369389
email_buffer
370390
.write(
@@ -444,15 +464,16 @@ async fn migrate_external_ids(
444464
.into_extract_localpart(synapse_user_id.clone())?
445465
.to_owned();
446466
let Some(user_infos) = state.users.get(username.as_str()).copied() else {
447-
if is_likely_appservice(&username) {
448-
continue;
449-
}
450467
return Err(Error::MissingUserFromDependentTable {
451468
table: "user_external_ids".to_owned(),
452469
user: synapse_user_id,
453470
});
454471
};
455472

473+
if user_infos.flags.is_appservice() {
474+
continue;
475+
}
476+
456477
let Some(&upstream_provider_id) = state.provider_id_mapping.get(&auth_provider) else {
457478
return Err(Error::MissingAuthProviderMapping {
458479
synapse_id: auth_provider,
@@ -534,16 +555,16 @@ async fn migrate_devices(
534555
.into_extract_localpart(synapse_user_id.clone())?
535556
.to_owned();
536557
let Some(user_infos) = state.users.get(username.as_str()).copied() else {
537-
if is_likely_appservice(&username) {
538-
continue;
539-
}
540558
return Err(Error::MissingUserFromDependentTable {
541559
table: "devices".to_owned(),
542560
user: synapse_user_id,
543561
});
544562
};
545563

546-
if user_infos.flags.is_deactivated() || user_infos.flags.is_guest() {
564+
if user_infos.flags.is_deactivated()
565+
|| user_infos.flags.is_guest()
566+
|| user_infos.flags.is_appservice()
567+
{
547568
continue;
548569
}
549570

@@ -663,16 +684,16 @@ async fn migrate_unrefreshable_access_tokens(
663684
.into_extract_localpart(synapse_user_id.clone())?
664685
.to_owned();
665686
let Some(user_infos) = state.users.get(username.as_str()).copied() else {
666-
if is_likely_appservice(&username) {
667-
continue;
668-
}
669687
return Err(Error::MissingUserFromDependentTable {
670688
table: "access_tokens".to_owned(),
671689
user: synapse_user_id,
672690
});
673691
};
674692

675-
if user_infos.flags.is_deactivated() || user_infos.flags.is_guest() {
693+
if user_infos.flags.is_deactivated()
694+
|| user_infos.flags.is_guest()
695+
|| user_infos.flags.is_appservice()
696+
{
676697
continue;
677698
}
678699

@@ -806,16 +827,16 @@ async fn migrate_refreshable_token_pairs(
806827
.into_extract_localpart(synapse_user_id.clone())?
807828
.to_owned();
808829
let Some(user_infos) = state.users.get(username.as_str()).copied() else {
809-
if is_likely_appservice(&username) {
810-
continue;
811-
}
812830
return Err(Error::MissingUserFromDependentTable {
813831
table: "refresh_tokens".to_owned(),
814832
user: synapse_user_id,
815833
});
816834
};
817835

818-
if user_infos.flags.is_deactivated() || user_infos.flags.is_guest() {
836+
if user_infos.flags.is_deactivated()
837+
|| user_infos.flags.is_guest()
838+
|| user_infos.flags.is_appservice()
839+
{
819840
continue;
820841
}
821842

@@ -917,33 +938,3 @@ fn transform_user(
917938

918939
Ok((new_user, mas_password))
919940
}
920-
921-
/// Returns true if and only if the given localpart looks like it would belong
922-
/// to an application service user.
923-
/// The rule here is that it must start with an underscore.
924-
/// Synapse reserves these by default, but there is no hard rule prohibiting
925-
/// other namespaces from being reserved, so this is not a robust check.
926-
// TODO replace with a more robust mechanism, if we even care about this sanity check
927-
// e.g. read application service registration files.
928-
#[inline]
929-
fn is_likely_appservice(localpart: &str) -> bool {
930-
// HACK(matrix.org): These are the namespaces we use on matrix.org
931-
localpart.starts_with('_')
932-
|| localpart.starts_with("freenode_") // Freenode IRC bridge
933-
|| localpart.starts_with("slack_") // Slack bridge
934-
|| localpart.starts_with("torn_") // Torn IRC bridge
935-
|| localpart.starts_with("gitter_") // Gitter bridge
936-
|| localpart.starts_with("mozilla_") // Mozilla IRC bridge
937-
|| localpart.starts_with("w3c_") // W3C IRC bridge
938-
|| localpart.starts_with("fs_") // VoIP conference AS
939-
// HACK(matrix.org): Sender localparts of those appservices
940-
|| localpart == "bifrost"
941-
|| localpart == "appservice-irc"
942-
|| localpart == "oftc-irc"
943-
|| localpart == "slackbot"
944-
|| localpart == "snoonet-irc"
945-
|| localpart == "torn-irc"
946-
|| localpart == "scalar"
947-
|| localpart == "gitterbot"
948-
|| localpart == "mozilla-irc"
949-
}

crates/syn2mas/src/synapse_reader/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ pub struct SynapseUser {
192192
/// account!
193193
pub is_guest: SynapseBool,
194194
// TODO do we care about upgrade_ts (users who upgraded from guest accounts to real accounts)
195+
/// The ID of the appservice that created this user, if any.
196+
pub appservice_id: Option<String>,
195197
}
196198

197199
/// Row of the `user_threepids` table in Synapse.
@@ -436,9 +438,8 @@ impl<'conn> SynapseReader<'conn> {
436438
sqlx::query_as(
437439
"
438440
SELECT
439-
name, password_hash, admin, deactivated, creation_ts, is_guest
441+
name, password_hash, admin, deactivated, creation_ts, is_guest, appservice_id
440442
FROM users
441-
WHERE appservice_id IS NULL
442443
",
443444
)
444445
.fetch(&mut *self.txn)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,6 @@ expression: users
2222
is_guest: SynapseBool(
2323
false,
2424
),
25+
appservice_id: None,
2526
},
2627
}

0 commit comments

Comments
 (0)