Add matrix.secret_file config option (element-hq#4840)

Author: sandhose

crates/cli/src/commands/doctor.rs

@@ -43,8 +43,8 @@ impl Options {
             r"The homeserver host in the config (`matrix.homeserver`) is not a valid domain.
 See {DOCS_BASE}/setup/homeserver.html",
         )?;
+        let admin_token = config.matrix.secret().await?;
         let hs_api = config.matrix.endpoint;
-        let admin_token = config.matrix.secret;
 
         if !issuer.starts_with("https://") {
             warn!(

crates/cli/src/commands/manage.rs

@@ -325,7 +325,8 @@ impl Options {
                 let matrix_config =
                     MatrixConfig::extract(figment).map_err(anyhow::Error::from_boxed)?;
                 let http_client = mas_http::reqwest_client();
-                let homeserver = homeserver_connection_from_config(&matrix_config, http_client);
+                let homeserver =
+                    homeserver_connection_from_config(&matrix_config, http_client).await?;
                 let mut conn = database_connection_from_config(&database_config).await?;
                 let txn = conn.begin().await?;
                 let mut repo = PgRepository::from_conn(txn);
@@ -612,7 +613,8 @@ impl Options {
                     MatrixConfig::extract(figment).map_err(anyhow::Error::from_boxed)?;
 
                 let password_manager = password_manager_from_config(&password_config).await?;
-                let homeserver = homeserver_connection_from_config(&matrix_config, http_client);
+                let homeserver =
+                    homeserver_connection_from_config(&matrix_config, http_client).await?;
                 let mut conn = database_connection_from_config(&database_config).await?;
                 let txn = conn.begin().await?;
                 let mut repo = PgRepository::from_conn(txn);

crates/cli/src/commands/server.rs

@@ -167,7 +167,7 @@ impl Options {
         let http_client = mas_http::reqwest_client();
 
         let homeserver_connection =
-            homeserver_connection_from_config(&config.matrix, http_client.clone());
+            homeserver_connection_from_config(&config.matrix, http_client.clone()).await?;
 
         if !self.no_worker {
             let mailer = mailer_from_config(&config.email, &templates)?;

crates/cli/src/commands/worker.rs

@@ -59,7 +59,7 @@ impl Options {
         test_mailer_in_background(&mailer, Duration::from_secs(30));
 
         let http_client = mas_http::reqwest_client();
-        let conn = homeserver_connection_from_config(&config.matrix, http_client);
+        let conn = homeserver_connection_from_config(&config.matrix, http_client).await?;
 
         drop(config);
 

crates/cli/src/util.rs

@@ -464,36 +464,36 @@ pub async fn load_policy_factory_dynamic_data(
 
 /// Create a clonable, type-erased [`HomeserverConnection`] from the
 /// configuration
-pub fn homeserver_connection_from_config(
+pub async fn homeserver_connection_from_config(
     config: &MatrixConfig,
     http_client: reqwest::Client,
-) -> Arc<dyn HomeserverConnection> {
-    match config.kind {
+) -> anyhow::Result<Arc<dyn HomeserverConnection>> {
+    Ok(match config.kind {
         HomeserverKind::Synapse | HomeserverKind::SynapseModern => {
             Arc::new(SynapseConnection::new(
                 config.homeserver.clone(),
                 config.endpoint.clone(),
-                config.secret.clone(),
+                config.secret().await?,
                 http_client,
             ))
         }
         HomeserverKind::SynapseLegacy => Arc::new(LegacySynapseConnection::new(
             config.homeserver.clone(),
             config.endpoint.clone(),
-            config.secret.clone(),
+            config.secret().await?,
             http_client,
         )),
         HomeserverKind::SynapseReadOnly => {
             let connection = SynapseConnection::new(
                 config.homeserver.clone(),
                 config.endpoint.clone(),
-                config.secret.clone(),
+                config.secret().await?,
                 http_client,
             );
             let readonly = ReadOnlyHomeserverConnection::new(connection);
             Arc::new(readonly)
         }
-    }
+    })
 }
 
 #[cfg(test)]

crates/config/src/sections/matrix.rs

@@ -4,6 +4,8 @@
 // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 // Please see LICENSE files in the repository root for full details.
 
+use anyhow::bail;
+use camino::Utf8PathBuf;
 use rand::{
     Rng,
     distributions::{Alphanumeric, DistString},
@@ -44,6 +46,54 @@ pub enum HomeserverKind {
     SynapseModern,
 }
 
+/// Shared secret between MAS and the homeserver.
+///
+/// It either holds the secret value directly or references a file where the
+/// secret is stored.
+#[derive(Clone, Debug)]
+pub enum Secret {
+    File(Utf8PathBuf),
+    Value(String),
+}
+
+/// Secret fields as serialized in JSON.
+#[derive(JsonSchema, Serialize, Deserialize, Clone, Debug)]
+struct SecretRaw {
+    #[schemars(with = "Option<String>")]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    secret_file: Option<Utf8PathBuf>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    secret: Option<String>,
+}
+
+impl TryFrom<SecretRaw> for Secret {
+    type Error = anyhow::Error;
+
+    fn try_from(value: SecretRaw) -> Result<Self, Self::Error> {
+        match (value.secret, value.secret_file) {
+            (None, None) => bail!("Missing `secret` or `secret_file`"),
+            (None, Some(path)) => Ok(Secret::File(path)),
+            (Some(secret), None) => Ok(Secret::Value(secret)),
+            (Some(_), Some(_)) => bail!("Cannot specify both `secret` and `secret_file`"),
+        }
+    }
+}
+
+impl From<Secret> for SecretRaw {
+    fn from(value: Secret) -> Self {
+        match value {
+            Secret::File(path) => SecretRaw {
+                secret_file: Some(path),
+                secret: None,
+            },
+            Secret::Value(secret) => SecretRaw {
+                secret_file: None,
+                secret: Some(secret),
+            },
+        }
+    }
+}
+
 /// Configuration related to the Matrix homeserver
 #[serde_as]
 #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
@@ -57,7 +107,10 @@ pub struct MatrixConfig {
     pub homeserver: String,
 
     /// Shared secret to use for calls to the admin API
-    pub secret: String,
+    #[schemars(with = "SecretRaw")]
+    #[serde_as(as = "serde_with::TryFromInto<SecretRaw>")]
+    #[serde(flatten)]
+    pub secret: Secret,
 
     /// The base URL of the homeserver's client API
     #[serde(default = "default_endpoint")]
@@ -69,14 +122,28 @@ impl ConfigurationSection for MatrixConfig {
 }
 
 impl MatrixConfig {
+    /// Returns the shared secret.
+    ///
+    /// If `secret_file` was given, the secret is read from that file.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error when the shared secret could not be read from file.
+    pub async fn secret(&self) -> anyhow::Result<String> {
+        Ok(match &self.secret {
+            Secret::File(path) => tokio::fs::read_to_string(path).await?,
+            Secret::Value(secret) => secret.clone(),
+        })
+    }
+
     pub(crate) fn generate<R>(mut rng: R) -> Self
     where
         R: Rng + Send,
     {
         Self {
             kind: HomeserverKind::default(),
             homeserver: default_homeserver(),
-            secret: Alphanumeric.sample_string(&mut rng, 32),
+            secret: Secret::Value(Alphanumeric.sample_string(&mut rng, 32)),
             endpoint: default_endpoint(),
         }
     }
@@ -85,7 +152,7 @@ impl MatrixConfig {
         Self {
             kind: HomeserverKind::default(),
             homeserver: default_homeserver(),
-            secret: "test".to_owned(),
+            secret: Secret::Value("test".to_owned()),
             endpoint: default_endpoint(),
         }
     }
@@ -97,29 +164,68 @@ mod tests {
         Figment, Jail,
         providers::{Format, Yaml},
     };
+    use tokio::{runtime::Handle, task};
 
     use super::*;
 
-    #[test]
-    fn load_config() {
-        Jail::expect_with(|jail| {
-            jail.create_file(
-                "config.yaml",
-                r"
-                    matrix:
-                      homeserver: matrix.org
-                      secret: test
-                ",
-            )?;
-
-            let config = Figment::new()
-                .merge(Yaml::file("config.yaml"))
-                .extract_inner::<MatrixConfig>("matrix")?;
-
-            assert_eq!(&config.homeserver, "matrix.org");
-            assert_eq!(&config.secret, "test");
-
-            Ok(())
-        });
+    #[tokio::test]
+    async fn load_config() {
+        task::spawn_blocking(|| {
+            Jail::expect_with(|jail| {
+                jail.create_file(
+                    "config.yaml",
+                    r"
+                        matrix:
+                          homeserver: matrix.org
+                          secret_file: secret
+                    ",
+                )?;
+                jail.create_file("secret", r"m472!x53c237")?;
+
+                let config = Figment::new()
+                    .merge(Yaml::file("config.yaml"))
+                    .extract_inner::<MatrixConfig>("matrix")?;
+
+                Handle::current().block_on(async move {
+                    assert_eq!(&config.homeserver, "matrix.org");
+                    assert!(matches!(config.secret, Secret::File(ref p) if p == "secret"));
+                    assert_eq!(config.secret().await.unwrap(), "m472!x53c237");
+                });
+
+                Ok(())
+            });
+        })
+        .await
+        .unwrap();
+    }
+
+    #[tokio::test]
+    async fn load_config_inline_secrets() {
+        task::spawn_blocking(|| {
+            Jail::expect_with(|jail| {
+                jail.create_file(
+                    "config.yaml",
+                    r"
+                        matrix:
+                          homeserver: matrix.org
+                          secret: m472!x53c237
+                    ",
+                )?;
+
+                let config = Figment::new()
+                    .merge(Yaml::file("config.yaml"))
+                    .extract_inner::<MatrixConfig>("matrix")?;
+
+                Handle::current().block_on(async move {
+                    assert_eq!(&config.homeserver, "matrix.org");
+                    assert!(matches!(config.secret, Secret::Value(ref v) if v == "m472!x53c237"));
+                    assert_eq!(config.secret().await.unwrap(), "m472!x53c237");
+                });
+
+                Ok(())
+            });
+        })
+        .await
+        .unwrap();
     }
 }

docs/config.schema.json

@@ -1677,9 +1677,6 @@
     "MatrixConfig": {
       "description": "Configuration related to the Matrix homeserver",
       "type": "object",
-      "required": [
-        "secret"
-      ],
       "properties": {
         "kind": {
           "description": "The kind of homeserver it is.",
@@ -1695,15 +1692,17 @@
           "default": "localhost:8008",
           "type": "string"
         },
-        "secret": {
-          "description": "Shared secret to use for calls to the admin API",
-          "type": "string"
-        },
         "endpoint": {
           "description": "The base URL of the homeserver's client API",
           "default": "http://localhost:8008/",
           "type": "string",
           "format": "uri"
+        },
+        "secret_file": {
+          "type": "string"
+        },
+        "secret": {
+          "type": "string"
         }
       }
     },

docs/reference/configuration.md

@@ -135,7 +135,9 @@ matrix:
 
   # Shared secret used to authenticate the service to the homeserver
   # This must be of high entropy, because leaking this secret would allow anyone to perform admin actions on the homeserver
-  secret: "SomeRandomSecret"
+  secret_file: /path/to/secret/file
+  # Alternatively, the shared secret can be passed inline.
+  # secret: "SomeRandomSecret"
 
   # URL to which the homeserver is accessible from the service
   endpoint: "http://localhost:8008"