Add pre-migration checks to syn2mas (#3805)

This matches or exceeds `advisor.mts` from the old tool.

Co-authored-by: Quentin Gliech <[email protected]>

Author: reivilibre

Cargo.lock

@@ -52,7 +52,8 @@ enum Subcommand {
 
     /// Migrate from Synapse's built-in auth system to MAS.
     #[clap(name = "syn2mas")]
-    Syn2Mas(self::syn2mas::Options),
+    // Box<> is to work around a 'large size difference between variants' lint
+    Syn2Mas(Box<self::syn2mas::Options>),
 }
 
 #[derive(Parser, Debug)]

crates/cli/src/commands/mod.rs

@@ -1,16 +1,23 @@
 use std::process::ExitCode;
 
 use anyhow::Context;
+use camino::Utf8PathBuf;
 use clap::Parser;
 use figment::Figment;
-use mas_config::{ConfigurationSectionExt, DatabaseConfig};
+use mas_config::{ConfigurationSection, ConfigurationSectionExt, DatabaseConfig, MatrixConfig};
 use rand::thread_rng;
-use sqlx::{Connection, Either, PgConnection};
-use syn2mas::{LockedMasDatabase, MasWriter, SynapseReader};
+use sqlx::{postgres::PgConnectOptions, Connection, Either, PgConnection};
+use syn2mas::{synapse_config, LockedMasDatabase, MasWriter, SynapseReader};
 use tracing::{error, warn};
 
 use crate::util::database_connection_from_config;
 
+/// The exit code used by `syn2mas check` and `syn2mas migrate` when there are errors preventing migration.
+const EXIT_CODE_CHECK_ERRORS: u8 = 10;
+
+/// The exit code used by `syn2mas check` when there are warnings which should be considered prior to migration.
+const EXIT_CODE_CHECK_WARNINGS: u8 = 11;
+
 #[derive(Parser, Debug)]
 pub(super) struct Options {
     #[command(subcommand)]
@@ -22,11 +29,37 @@ pub(super) struct Options {
     /// If you want to migrate from Synapse to MAS today, please use the Node.js-based tool in the MAS repository.
     #[clap(long = "i-swear-i-am-just-testing-in-a-staging-environment")]
     experimental_accepted: bool,
+
+    /// Path to the Synapse configuration (in YAML format).
+    /// May be specified multiple times if multiple Synapse configuration files are in use.
+    #[clap(long = "synapse-config")]
+    synapse_configuration_files: Vec<Utf8PathBuf>,
+
+    /// Override the Synapse database URI.
+    /// syn2mas normally loads the Synapse database connection details from the Synapse configuration.
+    /// However, it may sometimes be necessary to override the database URI and in that case this flag can be used.
+    ///
+    /// Should be a connection URI of the following general form:
+    /// ```text
+    /// postgresql://[user[:password]@][host][:port][/dbname][?param1=value1&...]
+    /// ```
+    /// To use a UNIX socket at a custom path, the host should be a path to a socket, but in the URI string
+    /// it must be URI-encoded by replacing `/` with `%2F`.
+    ///
+    /// Finally, any missing values will be loaded from the libpq-compatible environment variables
+    /// `PGHOST`, `PGPORT`, `PGUSER`, `PGDATABASE`, `PGPASSWORD`, etc.
+    /// It is valid to specify the URL `postgresql:` and configure all values through those environment variables.
+    #[clap(long = "synapse-database-uri")]
+    synapse_database_uri: Option<PgConnectOptions>,
 }
 
 #[derive(Parser, Debug)]
 enum Subcommand {
+    /// Check the setup for potential problems before running a migration.
+    ///
+    /// It is OK for Synapse to be online during these checks.
     Check,
+    /// Perform a migration. Synapse must be offline during this process.
     Migrate,
 }
 
@@ -41,8 +74,26 @@ impl Options {
             return Ok(ExitCode::FAILURE);
         }
 
-        // TODO allow configuring the synapse database location
-        let mut syn_conn = PgConnection::connect("postgres:///fakesyn").await.unwrap();
+        if self.synapse_configuration_files.is_empty() {
+            error!("Please specify the path to the Synapse configuration file(s).");
+            return Ok(ExitCode::FAILURE);
+        }
+
+        let synapse_config = synapse_config::Config::load(&self.synapse_configuration_files)
+            .context("Failed to load Synapse configuration")?;
+
+        // Establish a connection to Synapse's Postgres database
+        let syn_connection_options = if let Some(db_override) = self.synapse_database_uri {
+            db_override
+        } else {
+            synapse_config
+                .database
+                .to_sqlx_postgres()
+                .context("Synapse configuration does not use Postgres, cannot migrate.")?
+        };
+        let mut syn_conn = PgConnection::connect_with(&syn_connection_options)
+            .await
+            .context("could not connect to Synapse Postgres database")?;
 
         let config = DatabaseConfig::extract_or_default(figment)?;
 
@@ -57,27 +108,79 @@ impl Options {
             return Ok(ExitCode::FAILURE);
         };
 
+        // Check configuration
+        let (mut check_warnings, mut check_errors) = syn2mas::synapse_config_check(&synapse_config);
+        {
+            let (extra_warnings, extra_errors) =
+                syn2mas::synapse_config_check_against_mas_config(&synapse_config, figment).await?;
+            check_warnings.extend(extra_warnings);
+            check_errors.extend(extra_errors);
+        }
+
+        // Check databases
         syn2mas::mas_pre_migration_checks(&mut mas_connection).await?;
-        syn2mas::synapse_pre_migration_checks(&mut syn_conn).await?;
+        {
+            let (extra_warnings, extra_errors) =
+                syn2mas::synapse_database_check(&mut syn_conn, &synapse_config, figment).await?;
+            check_warnings.extend(extra_warnings);
+            check_errors.extend(extra_errors);
+        }
+
+        // Display errors and warnings
+        if !check_errors.is_empty() {
+            eprintln!("===== Errors =====");
+            eprintln!("These issues prevent migrating from Synapse to MAS right now:\n");
+            for error in &check_errors {
+                eprintln!("• {error}\n");
+            }
+        }
+        if !check_warnings.is_empty() {
+            eprintln!("===== Warnings =====");
+            eprintln!("These potential issues should be considered before migrating from Synapse to MAS right now:\n");
+            for warning in &check_warnings {
+                eprintln!("• {warning}\n");
+            }
+        }
 
-        let mut reader = SynapseReader::new(&mut syn_conn, true).await?;
-        let mut writer_mas_connections = Vec::with_capacity(NUM_WRITER_CONNECTIONS);
-        for _ in 0..NUM_WRITER_CONNECTIONS {
-            writer_mas_connections.push(database_connection_from_config(&config).await?);
+        // Do not proceed if there are any errors
+        if !check_errors.is_empty() {
+            return Ok(ExitCode::from(EXIT_CODE_CHECK_ERRORS));
         }
-        let mut writer = MasWriter::new(mas_connection, writer_mas_connections).await?;
 
-        // TODO is this rng ok?
-        #[allow(clippy::disallowed_methods)]
-        let mut rng = thread_rng();
+        match self.subcommand {
+            Subcommand::Check => {
+                if !check_warnings.is_empty() {
+                    return Ok(ExitCode::from(EXIT_CODE_CHECK_WARNINGS));
+                }
 
-        // TODO progress reporting
-        // TODO allow configuring the server name
-        syn2mas::migrate(&mut reader, &mut writer, "matrix.org", &mut rng).await?;
+                println!("Check completed successfully with no errors or warnings.");
 
-        reader.finish().await?;
-        writer.finish().await?;
+                Ok(ExitCode::SUCCESS)
+            }
+            Subcommand::Migrate => {
+                // TODO how should we handle warnings at this stage?
 
-        Ok(ExitCode::SUCCESS)
+                let mut reader = SynapseReader::new(&mut syn_conn, true).await?;
+                let mut writer_mas_connections = Vec::with_capacity(NUM_WRITER_CONNECTIONS);
+                for _ in 0..NUM_WRITER_CONNECTIONS {
+                    writer_mas_connections.push(database_connection_from_config(&config).await?);
+                }
+                let mut writer = MasWriter::new(mas_connection, writer_mas_connections).await?;
+
+                // TODO is this rng ok?
+                #[allow(clippy::disallowed_methods)]
+                let mut rng = thread_rng();
+
+                // TODO progress reporting
+                let mas_matrix = MatrixConfig::extract(figment)?;
+                syn2mas::migrate(&mut reader, &mut writer, &mas_matrix.homeserver, &mut rng)
+                    .await?;
+
+                reader.finish().await?;
+                writer.finish().await?;
+
+                Ok(ExitCode::SUCCESS)
+            }
+        }
     }
 }

crates/cli/src/commands/syn2mas.rs

@@ -51,7 +51,7 @@ pub use self::{
         ClaimsImports as UpstreamOAuth2ClaimsImports, DiscoveryMode as UpstreamOAuth2DiscoveryMode,
         EmailImportPreference as UpstreamOAuth2EmailImportPreference,
         ImportAction as UpstreamOAuth2ImportAction, PkceMethod as UpstreamOAuth2PkceMethod,
-        ResponseMode as UpstreamOAuth2ResponseMode,
+        Provider as UpstreamOAuth2Provider, ResponseMode as UpstreamOAuth2ResponseMode,
         TokenAuthMethod as UpstreamOAuth2TokenAuthMethod, UpstreamOAuth2Config,
     },
 };

crates/config/src/sections/mod.rs

@@ -179,7 +179,7 @@ fn default_bcrypt_cost() -> Option<u32> {
 }
 
 /// A hashing algorithm
-#[derive(Debug, Clone, Copy, Serialize, Deserialize, JsonSchema)]
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
 #[serde(rename_all = "lowercase")]
 pub enum Algorithm {
     /// bcrypt

crates/config/src/sections/passwords.rs

@@ -391,6 +391,7 @@ pub struct SignInWithApple {
     pub key_id: String,
 }
 
+/// Configuration for one upstream OAuth 2 provider.
 #[skip_serializing_none]
 #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
 pub struct Provider {
@@ -537,4 +538,21 @@ pub struct Provider {
     /// Orders of the keys are not preserved.
     #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
     pub additional_authorization_parameters: BTreeMap<String, String>,
+
+    /// The ID of the provider that was used by Synapse.
+    /// In order to perform a Synapse-to-MAS migration, this must be specified.
+    ///
+    /// ## For providers that used OAuth 2.0 or OpenID Connect in Synapse
+    ///
+    /// ### For `oidc_providers`:
+    /// This should be specified as `oidc-` followed by the ID that was
+    /// configured as `idp_id` in one of the `oidc_providers` in the Synapse
+    /// configuration.
+    /// For example, if Synapse's configuration contained `idp_id: wombat` for
+    /// this provider, then specify `oidc-wombat` here.
+    ///
+    /// ### For `oidc_config` (legacy):
+    /// Specify `oidc` here.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub synapse_idp_id: Option<String>,
 }

crates/config/src/sections/upstream_oauth2.rs

@@ -10,6 +10,10 @@ repository.workspace = true
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+anyhow.workspace = true
+camino.workspace = true
+figment.workspace = true
+serde.workspace = true
 thiserror.workspace = true
 thiserror-ext.workspace = true
 tokio.workspace = true
@@ -30,5 +34,7 @@ anyhow.workspace = true
 insta.workspace = true
 serde.workspace = true
 
+mas-config.workspace = true
+
 [lints]
 workspace = true

crates/syn2mas/Cargo.toml

@@ -6,11 +6,13 @@
 mod mas_writer;
 mod synapse_reader;
 
-mod checks;
 mod migration;
 
-pub use self::checks::synapse_pre_migration_checks;
 pub use self::mas_writer::locking::LockedMasDatabase;
 pub use self::mas_writer::{checks::mas_pre_migration_checks, MasWriter};
 pub use self::migration::migrate;
+pub use self::synapse_reader::checks::{
+    synapse_config_check, synapse_config_check_against_mas_config, synapse_database_check,
+};
+pub use self::synapse_reader::config as synapse_config;
 pub use self::synapse_reader::SynapseReader;

crates/syn2mas/src/checks.rs

@@ -207,6 +207,12 @@ pub struct MasNewUserPassword {
     pub created_at: DateTime<Utc>,
 }
 
+/// The 'version' of the password hashing scheme used for passwords when they are
+/// migrated from Synapse to MAS.
+/// This is version 1, as in the previous syn2mas script.
+// TODO hardcoding version to `1` may not be correct long-term?
+pub const MIGRATED_PASSWORD_VERSION: u16 = 1;
+
 /// List of all MAS tables that are written to by syn2mas.
 pub const MAS_TABLES_AFFECTED_BY_MIGRATION: &[&str] = &["users", "user_passwords"];
 
@@ -578,8 +584,7 @@ impl<'conn> MasWriter<'conn> {
                 user_ids.push(user_id);
                 hashed_passwords.push(hashed_password);
                 created_ats.push(created_at);
-            // TODO hardcoding version to `1` may not be correct long-term?
-                versions.push(1);
+                versions.push(MIGRATED_PASSWORD_VERSION.into());
             }
 
             sqlx::query!(