From 6a28950d8efb9e25791831bd5305f831a7e968ab Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Wed, 6 Aug 2025 15:21:30 +0200 Subject: [PATCH 1/4] Add secrets.keys_dir config option --- crates/config/src/sections/secrets.rs | 216 +++++++++++++++++++++++++- docs/config.schema.json | 19 ++- docs/reference/configuration.md | 19 ++- 3 files changed, 237 insertions(+), 17 deletions(-) diff --git a/crates/config/src/sections/secrets.rs b/crates/config/src/sections/secrets.rs index ea2463167..ac93a7abc 100644 --- a/crates/config/src/sections/secrets.rs +++ b/crates/config/src/sections/secrets.rs @@ -244,6 +244,86 @@ impl From for EncryptionRaw { } } +/// Description of signing keys. +/// +/// It either holds the key config values directly or references a directory +/// where each file contains a key. +#[derive(Debug, Clone)] +pub enum Keys { + Values(Vec), + Directory(Utf8PathBuf), +} + +impl Keys { + /// Returns a list of key configs. + /// + /// If `keys_dir` was given, the keys are read from file. + async fn key_configs(&self) -> anyhow::Result> { + match self { + Keys::Values(key_configs) => Ok(key_configs.clone()), + Keys::Directory(path) => key_configs_from_path(path).await, + } + } +} + +/// Reads all keys from the given directory. +async fn key_configs_from_path(path: &Utf8PathBuf) -> anyhow::Result> { + let mut result = vec![]; + let mut read_dir = tokio::fs::read_dir(path).await?; + while let Some(dir_entry) = read_dir.next_entry().await? { + if !dir_entry.path().is_file() { + continue; + } + result.push(KeyConfig { + kid: None, + password: None, + key: Key::File(dir_entry.path().try_into()?), + }); + } + Ok(result) +} + +#[serde_as] +#[derive(JsonSchema, Serialize, Deserialize, Debug, Clone)] +struct KeysRaw { + /// List of private keys to use for signing and encrypting payloads. + #[serde(skip_serializing_if = "Option::is_none")] + keys: Option>, + + /// Directory of private keys to use for signing and encrypting payloads. + #[schemars(with = "Option")] + #[serde(skip_serializing_if = "Option::is_none")] + keys_dir: Option, +} + +impl TryFrom for Keys { + type Error = anyhow::Error; + + fn try_from(value: KeysRaw) -> Result { + match (value.keys, value.keys_dir) { + (None, None) => bail!("Missing `keys` or `keys_dir`"), + (None, Some(path)) => Ok(Keys::Directory(path)), + (Some(keys), None) => Ok(Keys::Values(keys)), + (Some(_), Some(_)) => bail!("Cannot specify both `keys` and `keys_dir`"), + } + } +} + +impl From for KeysRaw { + fn from(value: Keys) -> Self { + match value { + Keys::Directory(path) => KeysRaw { + keys_dir: Some(path), + keys: None, + }, + Keys::Values(keys) => KeysRaw { + keys_dir: None, + keys: Some(keys), + }, + } + } +} + /// Application secrets #[serde_as] #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] @@ -255,8 +335,10 @@ pub struct SecretsConfig { encryption: Encryption, /// List of private keys to use for signing and encrypting payloads - #[serde(default)] - keys: Vec, + #[schemars(with = "KeysRaw")] + #[serde_as(as = "serde_with::TryFromInto")] + #[serde(flatten)] + keys: Keys, } impl SecretsConfig { @@ -267,7 +349,8 @@ impl SecretsConfig { /// Returns an error when a key could not be imported #[tracing::instrument(name = "secrets.load", skip_all)] pub async fn key_store(&self) -> anyhow::Result { - let web_keys = try_join_all(self.keys.iter().map(KeyConfig::json_web_key)).await?; + let key_configs = self.keys.key_configs().await?; + let web_keys = try_join_all(key_configs.iter().map(KeyConfig::json_web_key)).await?; Ok(Keystore::new(JsonWebKeySet::new(web_keys))) } @@ -382,7 +465,7 @@ impl SecretsConfig { Ok(Self { encryption: Encryption::Value(Standard.sample(&mut rng)), - keys: vec![rsa_key, ec_p256_key, ec_p384_key, ec_k256_key], + keys: Keys::Values(vec![rsa_key, ec_p256_key, ec_p384_key, ec_k256_key]), }) } @@ -423,7 +506,7 @@ impl SecretsConfig { Self { encryption: Encryption::Value([0xEA; 32]), - keys: vec![rsa_key, ecdsa_key], + keys: Keys::Values(vec![rsa_key, ecdsa_key]), } } } @@ -439,6 +522,129 @@ mod tests { use super::*; + #[tokio::test] + async fn load_config() { + task::spawn_blocking(|| { + Jail::expect_with(|jail| { + jail.create_file( + "config.yaml", + indoc::indoc! {r" + secrets: + encryption_file: encryption + keys_dir: keys + "}, + )?; + jail.create_file("encryption", example_secret())?; + jail.create_dir("keys")?; + jail.create_file( + "keys/key1", + indoc::indoc! {r" + -----BEGIN RSA PRIVATE KEY----- + MIIJKQIBAAKCAgEA6oR6LXzJOziUxcRryonLTM5Xkfr9cYPCKvnwsWoAHfd2MC6Q + OCAWSQnNcNz5RTeQUcLEaA8sxQi64zpCwO9iH8y8COCaO8u9qGkOOuJwWnmPfeLs + cEwALEp0LZ67eSUPsMaz533bs4C8p+2UPMd+v7Td8TkkYoqgUrfYuT0bDTMYVsSe + wcNB5qsI7hDLf1t5FX6KU79/Asn1K3UYHTdN83mghOlM4zh1l1CJdtgaE1jAg4Ml + 1X8yG+cT+Ks8gCSGQfIAlVFV4fvvzmpokNKfwAI/b3LS2/ft4ZrK+RCTsWsjUu38 + Zr8jbQMtDznzBHMw1LoaHpwRNjbJZ7uA6x5ikbwz5NAlfCITTta6xYn8qvaBfiYJ + YyUFl0kIHm9Kh9V9p54WPMCFCcQx12deovKV82S6zxTeMflDdosJDB/uG9dT2qPt + wkpTD6xAOx5h59IhfiY0j4ScTl725GygVzyK378soP3LQ/vBixQLpheALViotodH + fJknsrelaISNkrnapZL3QE5C1SUoaUtMG9ovRz5HDpMx5ooElEklq7shFWDhZXbp + 2ndU5RPRCZO3Szop/Xhn2mNWQoEontFh79WIf+wS8TkJIRXhjtYBt3+s96z0iqSg + gDmE8BcP4lP1+TAUY1d7+QEhGCsTJa9TYtfDtNNfuYI9e3mq6LEpHYKWOvECAwEA + AQKCAgAlF60HaCGf50lzT6eePQCAdnEtWrMeyDCRgZTLStvCjEhk7d3LssTeP9mp + oe8fPomUv6c3BOds2/5LQFockABHd/y/CV9RA973NclAEQlPlhiBrb793Vd4VJJe + 6331dveDW0+ggVdFjfVzjhqQfnE9ZcsQ2JvjpiTI0Iv2cy7F01tke0GCSMgx8W1p + J2jjDOxwNOKGGoIT8S4roHVJnFy3nM4sbNtyDj+zHimP4uBE8m2zSgQAP60E8sia + 3+Ki1flnkXJRgQWCHR9cg5dkXfFRz56JmcdgxAHGWX2vD9XRuFi5nitPc6iTw8PV + u7GvS3+MC0oO+1pRkTAhOGv3RDK3Uqmy2zrMUuWkEsz6TVId6gPl7+biRJcP+aER + plJkeC9J9nSizbQPwErGByzoHGLjADgBs9hwqYkPcN38b6jR5S/VDQ+RncCyI87h + s/0pIs/fNlfw4LtpBrolP6g++vo6KUufmE3kRNN9dN4lNOoKjUGkcmX6MGnwxiw6 + NN/uEqf9+CKQele1XeUhRPNJc9Gv+3Ly5y/wEi6FjfVQmCK4hNrl3tvuZw+qkGbq + Au9Jhk7wV81An7fbhBRIXrwOY9AbOKNqUfY+wpKi5vyJFS1yzkFaYSTKTBspkuHW + pWbohO+KreREwaR5HOMK8tQMTLEAeE3taXGsQMJSJ15lRrLc7QKCAQEA68TV/R8O + C4p+vnGJyhcfDJt6+KBKWlroBy75BG7Dg7/rUXaj+MXcqHi+whRNXMqZchSwzUfS + B2WK/HrOBye8JLKDeA3B5TumJaF19vV7EY/nBF2QdRmI1r33Cp+RWUvAcjKa/v2u + KksV3btnJKXCu/stdAyTK7nU0on4qBzm5WZxuIJv6VMHLDNPFdCk+4gM8LuJ3ITU + l7XuZd4gXccPNj0VTeOYiMjIwxtNmE9RpCkTLm92Z7MI+htciGk1xvV0N4m1BXwA + 7qhl1nBgVuJyux4dEYFIeQNhLpHozkEz913QK2gDAHL9pAeiUYJntq4p8HNvfHiQ + vE3wTzil3aUFnwKCAQEA/qQm1Nx5By6an5UunrOvltbTMjsZSDnWspSQbX//j6mL + 2atQLe3y/Nr7E5SGZ1kFD9tgAHTuTGVqjvTqp5dBPw4uo146K2RJwuvaYUzNK26c + VoGfMfsI+/bfMfjFnEmGRARZdMr8cvhU+2m04hglsSnNGxsvvPdsiIbRaVDx+JvN + C5C281WlN0WeVd7zNTZkdyUARNXfCxBHQPuYkP5Mz2roZeYlJMWU04i8Cx0/SEuu + bhZQDaNTccSdPDFYcyDDlpqp+mN+U7m+yUPOkVpaxQiSYJZ+NOQsNcAVYfjzyY0E + /VP3s2GddjCJs0amf9SeW0LiMAHPgTp8vbMSRPVVbwKCAQEAmZsSd+llsys2TEmY + pivONN6PjbCRALE9foCiCLtJcmr1m4uaZRg0HScd0UB87rmoo2TLk9L5CYyksr4n + wQ2oTJhpgywjaYAlTVsWiiGBXv3MW1HCLijGuHHno+o2PmFWLpC93ufUMwXcZywT + lRLR/rs07+jJcbGO8OSnNpAt9sN5z+Zblz5a6/c5zVK0SpRnKehld2CrSXRkr8W6 + fJ6WUJYXbTmdRXDbLBJ7yYHUBQolzxkboZBJhvmQnec9/DQq1YxIfhw+Vz8rqjxo + 5/J9IWALPD5owz7qb/bsIITmoIFkgQMxAXfpvJaksEov3Bs4g8oRlpzOX4C/0j1s + Ay3irQKCAQEAwRJ/qufcEFkCvjsj1QsS+MC785shyUSpiE/izlO91xTLx+f/7EM9 + +QCkXK1B1zyE/Qft24rNYDmJOQl0nkuuGfxL2mzImDv7PYMM2reb3PGKMoEnzoKz + xi/h/YbNdnm9BvdxSH/cN+QYs2Pr1X5Pneu+622KnbHQphfq0fqg7Upchwdb4Faw + 5Z6wthVMvK0YMcppUMgEzOOz0w6xGEbowGAkA5cj1KTG+jjzs02ivNM9V5Utb5nF + 3D4iphAYK3rNMfTlKsejciIlCX+TMVyb9EdSjU+uM7ZJ2xtgWx+i4NA+10GCT42V + EZct4TORbN0ukK2+yH2m8yoAiOks0gJemwKCAQAMGROGt8O4HfhpUdOq01J2qvQL + m5oUXX8w1I95XcoAwCqb+dIan8UbCyl/79lbqNpQlHbRy3wlXzWwH9aHKsfPlCvk + 5dE1qrdMdQhLXwP109bRmTiScuU4zfFgHw3XgQhMFXxNp9pze197amLws0TyuBW3 + fupS4kM5u6HKCeBYcw2WP5ukxf8jtn29tohLBiA2A7NYtml9xTer6BBP0DTh+QUn + IJL6jSpuCNxBPKIK7p6tZZ0nMBEdAWMxglYm0bmHpTSd3pgu3ltCkYtDlDcTIaF0 + Q4k44lxUTZQYwtKUVQXBe4ZvaT/jIEMS7K5bsAy7URv/toaTaiEh1hguwSmf + -----END RSA PRIVATE KEY----- + "}, + )?; + jail.create_file( + "keys/key2", + indoc::indoc! {r" + -----BEGIN EC PRIVATE KEY----- + MHcCAQEEIKlZz/GnH0idVH1PnAF4HQNwRafgBaE2tmyN1wjfdOQqoAoGCCqGSM49 + AwEHoUQDQgAEHrgPeG+Mt8eahih1h4qaPjhl7jT25cdzBkg3dbVks6gBR2Rx4ug9 + h27LAir5RqxByHvua2XsP46rSTChof78uw== + -----END EC PRIVATE KEY----- + "}, + )?; + + let config = Figment::new() + .merge(Yaml::file("config.yaml")) + .extract_inner::("secrets")?; + + Handle::current().block_on(async move { + assert!( + matches!(config.encryption, Encryption::File(ref p) if p == "encryption") + ); + assert_eq!( + config.encryption().await.unwrap(), + [ + 0, 0, 17, 17, 34, 34, 51, 51, 68, 68, 85, 85, 102, 102, 119, 119, 136, + 136, 153, 153, 170, 170, 187, 187, 204, 204, 221, 221, 238, 238, 255, + 255 + ] + ); + + let mut key_config = config.keys.key_configs().await.unwrap(); + key_config.sort_by_key(|a| { + if let Key::File(p) = &a.key { + Some(p.clone()) + } else { + None + } + }); + let key_store = config.key_store().await.unwrap(); + + assert!(key_config[0].kid.is_none()); + assert!(matches!(&key_config[0].key, Key::File(p) if p == "keys/key1")); + assert!(key_store.iter().any(|k| k.kid() == Some("xmgGCzGtQFmhEOP0YAqBt-oZyVauSVMXcf4kwcgGZLc"))); + assert!(key_config[1].kid.is_none()); + assert!(matches!(&key_config[1].key, Key::File(p) if p == "keys/key2")); + assert!(key_store.iter().any(|k| k.kid() == Some("ONUCn80fsiISFWKrVMEiirNVr-QEvi7uQI0QH9q9q4o"))); + }); + + Ok(()) + }); + }) + .await + .unwrap(); + } + #[tokio::test] async fn load_config_inline_secrets() { task::spawn_blocking(|| { diff --git a/docs/config.schema.json b/docs/config.schema.json index ada9005c7..8a97cad13 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -1528,14 +1528,6 @@ "description": "Application secrets", "type": "object", "properties": { - "keys": { - "description": "List of private keys to use for signing and encrypting payloads", - "default": [], - "type": "array", - "items": { - "$ref": "#/definitions/KeyConfig" - } - }, "encryption_file": { "description": "File containing the encryption key for secure cookies.", "type": "string" @@ -1547,6 +1539,17 @@ ], "type": "string", "pattern": "[0-9a-fA-F]{64}" + }, + "keys": { + "description": "List of private keys to use for signing and encrypting payloads.", + "type": "array", + "items": { + "$ref": "#/definitions/KeyConfig" + } + }, + "keys_dir": { + "description": "Directory of private keys to use for signing and encrypting payloads.", + "type": "string" } } }, diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index b292aa332..1f5f4fd11 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -222,7 +222,7 @@ The secret is not updated when the content of the file changes. > Changing the encryption secret afterwards will lead to a loss of all encrypted > information in the database. -### `secrets.keys` +### Singing Keys The service can use a number of key types for signing. The following key types are supported: @@ -232,15 +232,26 @@ The following key types are supported: - ECDSA with the P-384 (`secp384r1`) curve - ECDSA with the K-256 (`secp256k1`) curve -Each entry in the list corresponds to one signing key used by MAS. -The key can either be specified inline (with the `key` property), -or loaded from a file (with the `key_file` property). The following key formats are supported: - PKCS#1 PEM or DER-encoded RSA private key - PKCS#8 PEM or DER-encoded RSA or ECDSA private key, encrypted or not - SEC1 PEM or DER-encoded ECDSA private key +The keys can be given as a directory path via `secrets.keys_dir` +or, alternatively, as an inline configuration list via `secrets.keys`. + +#### `secrets.keys_dir` + +Path to the directory containing MAS signing key files. +Only keys that don’t require a password are supported. + +#### `secrets.keys` + +Each entry in the list corresponds to one signing key used by MAS. +The key can either be specified inline (with the `key` property), +or loaded from a file (with the `key_file` property). + A [JWK Key ID] is automatically derived from each key. To override this default, set `kid` to a custom value. The `kid` can be any case-sensitive string value as long as it is unique to this list; From b5a0834faafa15c75451e89eba6d024e9e544b55 Mon Sep 17 00:00:00 2001 From: networkException Date: Sun, 10 Aug 2025 03:33:17 +0200 Subject: [PATCH 2/4] Add `upstream_oauth2.providers.[].client_secret_file` config option This patch factors out the previously introduced config wrapper for client secrets to also use it for upstream oauth providers. See a7e7c3caa18266f962c1135c1d4a42d5967d5896 --- crates/cli/src/sync.rs | 37 +++--- crates/config/src/sections/clients.rs | 67 +--------- crates/config/src/sections/mod.rs | 81 ++++++++++++ crates/config/src/sections/upstream_oauth2.rs | 118 +++++++++++++++++- .../syn2mas/src/synapse_reader/config/oidc.rs | 8 +- docs/config.schema.json | 9 +- docs/reference/configuration.md | 3 +- 7 files changed, 229 insertions(+), 94 deletions(-) diff --git a/crates/cli/src/sync.rs b/crates/cli/src/sync.rs index e66b3aa50..1aa8f7402 100644 --- a/crates/cli/src/sync.rs +++ b/crates/cli/src/sync.rs @@ -202,25 +202,24 @@ pub async fn config_sync( continue; } - let encrypted_client_secret = - if let Some(client_secret) = provider.client_secret.as_deref() { - Some(encrypter.encrypt_to_string(client_secret.as_bytes())?) - } else if let Some(mut siwa) = provider.sign_in_with_apple.clone() { - // if private key file is defined and not private key (raw), we populate the - // private key to hold the content of the private key file. - // private key (raw) takes precedence so both can be defined - // without issues - if siwa.private_key.is_none() - && let Some(private_key_file) = siwa.private_key_file.take() - { - let key = tokio::fs::read_to_string(private_key_file).await?; - siwa.private_key = Some(key); - } - let encoded = serde_json::to_vec(&siwa)?; - Some(encrypter.encrypt_to_string(&encoded)?) - } else { - None - }; + let encrypted_client_secret = if let Some(client_secret) = provider.client_secret { + Some(encrypter.encrypt_to_string(client_secret.value().await?.as_bytes())?) + } else if let Some(mut siwa) = provider.sign_in_with_apple.clone() { + // if private key file is defined and not private key (raw), we populate the + // private key to hold the content of the private key file. + // private key (raw) takes precedence so both can be defined + // without issues + if siwa.private_key.is_none() + && let Some(private_key_file) = siwa.private_key_file.take() + { + let key = tokio::fs::read_to_string(private_key_file).await?; + siwa.private_key = Some(key); + } + let encoded = serde_json::to_vec(&siwa)?; + Some(encrypter.encrypt_to_string(&encoded)?) + } else { + None + }; let discovery_mode = match provider.discovery_mode { mas_config::UpstreamOAuth2DiscoveryMode::Oidc => { diff --git a/crates/config/src/sections/clients.rs b/crates/config/src/sections/clients.rs index 0951ebba2..8b9120045 100644 --- a/crates/config/src/sections/clients.rs +++ b/crates/config/src/sections/clients.rs @@ -6,8 +6,6 @@ use std::ops::Deref; -use anyhow::bail; -use camino::Utf8PathBuf; use mas_iana::oauth::OAuthClientAuthenticationMethod; use mas_jose::jwk::PublicJsonWebKeySet; use schemars::JsonSchema; @@ -16,7 +14,7 @@ use serde_with::serde_as; use ulid::Ulid; use url::Url; -use super::ConfigurationSection; +use super::{ClientSecret, ClientSecretRaw, ConfigurationSection}; #[derive(JsonSchema, Serialize, Deserialize, Clone, Debug)] #[serde(rename_all = "snake_case")] @@ -31,66 +29,6 @@ impl From for JwksOrJwksUri { } } -/// Client secret config option. -/// -/// It either holds the client secret value directly or references a file where -/// the client secret is stored. -#[derive(Clone, Debug)] -pub enum ClientSecret { - File(Utf8PathBuf), - Value(String), -} - -/// Client secret fields as serialized in JSON. -#[derive(JsonSchema, Serialize, Deserialize, Clone, Debug)] -struct ClientSecretRaw { - /// Path to the file containing the client secret. The client secret is used - /// by the `client_secret_basic`, `client_secret_post` and - /// `client_secret_jwt` authentication methods. - #[schemars(with = "Option")] - #[serde(skip_serializing_if = "Option::is_none")] - client_secret_file: Option, - - /// Alternative to `client_secret_file`: Reads the client secret directly - /// from the config. - #[serde(skip_serializing_if = "Option::is_none")] - client_secret: Option, -} - -impl TryFrom for Option { - type Error = anyhow::Error; - - fn try_from(value: ClientSecretRaw) -> Result { - match (value.client_secret, value.client_secret_file) { - (None, None) => Ok(None), - (None, Some(path)) => Ok(Some(ClientSecret::File(path))), - (Some(client_secret), None) => Ok(Some(ClientSecret::Value(client_secret))), - (Some(_), Some(_)) => { - bail!("Cannot specify both `client_secret` and `client_secret_file`") - } - } - } -} - -impl From> for ClientSecretRaw { - fn from(value: Option) -> Self { - match value { - Some(ClientSecret::File(path)) => ClientSecretRaw { - client_secret_file: Some(path), - client_secret: None, - }, - Some(ClientSecret::Value(client_secret)) => ClientSecretRaw { - client_secret_file: None, - client_secret: Some(client_secret), - }, - None => ClientSecretRaw { - client_secret_file: None, - client_secret: None, - }, - } - } -} - /// Authentication method used by clients #[derive(JsonSchema, Serialize, Deserialize, Copy, Clone, Debug)] #[serde(rename_all = "snake_case")] @@ -273,8 +211,7 @@ impl ClientConfig { /// Returns an error when the client secret could not be read from file. pub async fn client_secret(&self) -> anyhow::Result> { Ok(match &self.client_secret { - Some(ClientSecret::File(path)) => Some(tokio::fs::read_to_string(path).await?), - Some(ClientSecret::Value(client_secret)) => Some(client_secret.clone()), + Some(client_secret) => Some(client_secret.value().await?), None => None, }) } diff --git a/crates/config/src/sections/mod.rs b/crates/config/src/sections/mod.rs index f992d8698..eb4ff2a44 100644 --- a/crates/config/src/sections/mod.rs +++ b/crates/config/src/sections/mod.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; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -303,3 +305,82 @@ impl ConfigurationSection for SyncConfig { Ok(()) } } + +/// Client secret config option. +/// +/// It either holds the client secret value directly or references a file where +/// the client secret is stored. +#[derive(Clone, Debug)] +pub enum ClientSecret { + /// Path to the file containing the client secret. + File(Utf8PathBuf), + + /// Client secret value. + Value(String), +} + +/// Client secret fields as serialized in JSON. +#[derive(JsonSchema, Serialize, Deserialize, Clone, Debug)] +pub struct ClientSecretRaw { + /// Path to the file containing the client secret. The client secret is used + /// by the `client_secret_basic`, `client_secret_post` and + /// `client_secret_jwt` authentication methods. + #[schemars(with = "Option")] + #[serde(skip_serializing_if = "Option::is_none")] + client_secret_file: Option, + + /// Alternative to `client_secret_file`: Reads the client secret directly + /// from the config. + #[serde(skip_serializing_if = "Option::is_none")] + client_secret: Option, +} + +impl ClientSecret { + /// Returns the client secret. + /// + /// If `client_secret_file` was given, the secret is read from that file. + /// + /// # Errors + /// + /// Returns an error when the client secret could not be read from file. + pub async fn value(&self) -> anyhow::Result { + Ok(match self { + ClientSecret::File(path) => tokio::fs::read_to_string(path).await?, + ClientSecret::Value(client_secret) => client_secret.clone(), + }) + } +} + +impl TryFrom for Option { + type Error = anyhow::Error; + + fn try_from(value: ClientSecretRaw) -> Result { + match (value.client_secret, value.client_secret_file) { + (None, None) => Ok(None), + (None, Some(path)) => Ok(Some(ClientSecret::File(path))), + (Some(client_secret), None) => Ok(Some(ClientSecret::Value(client_secret))), + (Some(_), Some(_)) => { + bail!("Cannot specify both `client_secret` and `client_secret_file`") + } + } + } +} + +impl From> for ClientSecretRaw { + fn from(value: Option) -> Self { + match value { + Some(ClientSecret::File(path)) => ClientSecretRaw { + client_secret_file: Some(path), + client_secret: None, + }, + Some(ClientSecret::Value(client_secret)) => ClientSecretRaw { + client_secret_file: None, + client_secret: Some(client_secret), + }, + None => ClientSecretRaw { + client_secret_file: None, + client_secret: None, + }, + } + } +} diff --git a/crates/config/src/sections/upstream_oauth2.rs b/crates/config/src/sections/upstream_oauth2.rs index 05f70cc67..53eae7a1b 100644 --- a/crates/config/src/sections/upstream_oauth2.rs +++ b/crates/config/src/sections/upstream_oauth2.rs @@ -10,11 +10,11 @@ use camino::Utf8PathBuf; use mas_iana::jose::JsonWebSignatureAlg; use schemars::JsonSchema; use serde::{Deserialize, Serialize, de::Error}; -use serde_with::skip_serializing_none; +use serde_with::{serde_as, skip_serializing_none}; use ulid::Ulid; use url::Url; -use crate::ConfigurationSection; +use crate::{ClientSecret, ClientSecretRaw, ConfigurationSection}; /// Upstream OAuth 2.0 providers configuration #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, Default)] @@ -475,6 +475,7 @@ impl OnBackchannelLogout { } /// Configuration for one upstream OAuth 2 provider. +#[serde_as] #[skip_serializing_none] #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct Provider { @@ -541,8 +542,10 @@ pub struct Provider { /// /// Used by the `client_secret_basic`, `client_secret_post`, and /// `client_secret_jwt` methods - #[serde(skip_serializing_if = "Option::is_none")] - pub client_secret: Option, + #[schemars(with = "ClientSecretRaw")] + #[serde_as(as = "serde_with::TryFromInto")] + #[serde(flatten)] + pub client_secret: Option, /// The method to authenticate the client with the provider pub token_endpoint_auth_method: TokenAuthMethod, @@ -656,3 +659,110 @@ pub struct Provider { #[serde(default, skip_serializing_if = "OnBackchannelLogout::is_default")] pub on_backchannel_logout: OnBackchannelLogout, } + +impl Provider { + /// Returns the client secret. + /// + /// If `client_secret_file` was given, the secret is read from that file. + /// + /// # Errors + /// + /// Returns an error when the client secret could not be read from file. + pub async fn client_secret(&self) -> anyhow::Result> { + Ok(match &self.client_secret { + Some(client_secret) => Some(client_secret.value().await?), + None => None, + }) + } +} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use figment::{ + Figment, Jail, + providers::{Format, Yaml}, + }; + use tokio::{runtime::Handle, task}; + + use super::*; + + #[tokio::test] + async fn load_config() { + task::spawn_blocking(|| { + Jail::expect_with(|jail| { + jail.create_file( + "config.yaml", + r#" + upstream_oauth2: + providers: + - id: 01GFWR28C4KNE04WG3HKXB7C9R + client_id: upstream-oauth2 + token_endpoint_auth_method: none + + - id: 01GFWR32NCQ12B8Z0J8CPXRRB6 + client_id: upstream-oauth2 + client_secret_file: secret + token_endpoint_auth_method: client_secret_basic + + - id: 01GFWR3WHR93Y5HK389H28VHZ9 + client_id: upstream-oauth2 + client_secret: c1!3n753c237 + token_endpoint_auth_method: client_secret_post + + - id: 01GFWR43R2ZZ8HX9CVBNW9TJWG + client_id: upstream-oauth2 + client_secret_file: secret + token_endpoint_auth_method: client_secret_jwt + + - id: 01GFWR4BNFDCC4QDG6AMSP1VRR + client_id: upstream-oauth2 + token_endpoint_auth_method: private_key_jwt + jwks: + keys: + - kid: "03e84aed4ef4431014e8617567864c4efaaaede9" + kty: "RSA" + alg: "RS256" + use: "sig" + e: "AQAB" + n: "ma2uRyBeSEOatGuDpCiV9oIxlDWix_KypDYuhQfEzqi_BiF4fV266OWfyjcABbam59aJMNvOnKW3u_eZM-PhMCBij5MZ-vcBJ4GfxDJeKSn-GP_dJ09rpDcILh8HaWAnPmMoi4DC0nrfE241wPISvZaaZnGHkOrfN_EnA5DligLgVUbrA5rJhQ1aSEQO_gf1raEOW3DZ_ACU3qhtgO0ZBG3a5h7BPiRs2sXqb2UCmBBgwyvYLDebnpE7AotF6_xBIlR-Cykdap3GHVMXhrIpvU195HF30ZoBU4dMd-AeG6HgRt4Cqy1moGoDgMQfbmQ48Hlunv9_Vi2e2CLvYECcBw" + + - kid: "d01c1abe249269f72ef7ca2613a86c9f05e59567" + kty: "RSA" + alg: "RS256" + use: "sig" + e: "AQAB" + n: "0hukqytPwrj1RbMYhYoepCi3CN5k7DwYkTe_Cmb7cP9_qv4ok78KdvFXt5AnQxCRwBD7-qTNkkfMWO2RxUMBdQD0ED6tsSb1n5dp0XY8dSWiBDCX8f6Hr-KolOpvMLZKRy01HdAWcM6RoL9ikbjYHUEW1C8IJnw3MzVHkpKFDL354aptdNLaAdTCBvKzU9WpXo10g-5ctzSlWWjQuecLMQ4G1mNdsR1LHhUENEnOvgT8cDkX0fJzLbEbyBYkdMgKggyVPEB1bg6evG4fTKawgnf0IDSPxIU-wdS9wdSP9ZCJJPLi5CEp-6t6rE_sb2dGcnzjCGlembC57VwpkUvyMw" + "#, + )?; + jail.create_file("secret", r"c1!3n753c237")?; + + let config = Figment::new() + .merge(Yaml::file("config.yaml")) + .extract_inner::("upstream_oauth2")?; + + assert_eq!(config.providers.len(), 5); + + assert_eq!( + config.providers[1].id, + Ulid::from_str("01GFWR32NCQ12B8Z0J8CPXRRB6").unwrap() + ); + + assert!(config.providers[0].client_secret.is_none()); + assert!(matches!(config.providers[1].client_secret, Some(ClientSecret::File(ref p)) if p == "secret")); + assert!(matches!(config.providers[2].client_secret, Some(ClientSecret::Value(ref v)) if v == "c1!3n753c237")); + assert!(matches!(config.providers[3].client_secret, Some(ClientSecret::File(ref p)) if p == "secret")); + assert!(config.providers[4].client_secret.is_none()); + + Handle::current().block_on(async move { + assert_eq!(config.providers[1].client_secret().await.unwrap().unwrap(), "c1!3n753c237"); + assert_eq!(config.providers[2].client_secret().await.unwrap().unwrap(), "c1!3n753c237"); + assert_eq!(config.providers[3].client_secret().await.unwrap().unwrap(), "c1!3n753c237"); + }); + + Ok(()) + }); + }).await.unwrap(); + } +} diff --git a/crates/syn2mas/src/synapse_reader/config/oidc.rs b/crates/syn2mas/src/synapse_reader/config/oidc.rs index 49ee59a71..09baba165 100644 --- a/crates/syn2mas/src/synapse_reader/config/oidc.rs +++ b/crates/syn2mas/src/synapse_reader/config/oidc.rs @@ -7,9 +7,9 @@ use std::{collections::BTreeMap, str::FromStr as _}; use chrono::{DateTime, Utc}; use mas_config::{ - UpstreamOAuth2ClaimsImports, UpstreamOAuth2DiscoveryMode, UpstreamOAuth2ImportAction, - UpstreamOAuth2OnBackchannelLogout, UpstreamOAuth2PkceMethod, UpstreamOAuth2ResponseMode, - UpstreamOAuth2TokenAuthMethod, + ClientSecret, UpstreamOAuth2ClaimsImports, UpstreamOAuth2DiscoveryMode, + UpstreamOAuth2ImportAction, UpstreamOAuth2OnBackchannelLogout, UpstreamOAuth2PkceMethod, + UpstreamOAuth2ResponseMode, UpstreamOAuth2TokenAuthMethod, }; use mas_iana::jose::JsonWebSignatureAlg; use oauth2_types::scope::{OPENID, Scope, ScopeToken}; @@ -328,7 +328,7 @@ impl OidcProvider { human_name: self.idp_name, brand_name: self.idp_brand, client_id, - client_secret: self.client_secret, + client_secret: self.client_secret.map(ClientSecret::Value), token_endpoint_auth_method, sign_in_with_apple: None, token_endpoint_auth_signing_alg: None, diff --git a/docs/config.schema.json b/docs/config.schema.json index f6256639b..eeb6992cc 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2159,8 +2159,15 @@ "description": "The client ID to use when authenticating with the provider", "type": "string" }, + "client_secret_file": { + "description": "Path to the file containing the client secret. The client secret is used\n by the `client_secret_basic`, `client_secret_post` and\n `client_secret_jwt` authentication methods.", + "type": [ + "string", + "null" + ] + }, "client_secret": { - "description": "The client secret to use when authenticating with the provider\n\n Used by the `client_secret_basic`, `client_secret_post`, and\n `client_secret_jwt` methods", + "description": "Alternative to `client_secret_file`: Reads the client secret directly\n from the config.", "type": [ "string", "null" diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index c5b69e38f..6f69e4f12 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -642,7 +642,8 @@ upstream_oauth2: # The client secret to use to authenticate to the provider # This is only used by the `client_secret_post`, `client_secret_basic` # and `client_secret_jwk` authentication methods - #client_secret: f4f6bb68a0269264877e9cb23b1856ab + client_secret_file: secret + # OR client_secret: f4f6bb68a0269264877e9cb23b1856ab # Which authentication method to use to authenticate to the provider # Supported methods are: From c9a8123335311d57966e9e71976429d208c73e5f Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Tue, 18 Nov 2025 19:00:11 +0100 Subject: [PATCH 3/4] Allow keys and keys_dir simultaneously --- crates/config/src/sections/secrets.rs | 154 ++++++++++++++------------ 1 file changed, 82 insertions(+), 72 deletions(-) diff --git a/crates/config/src/sections/secrets.rs b/crates/config/src/sections/secrets.rs index 8583e776f..98feb672d 100644 --- a/crates/config/src/sections/secrets.rs +++ b/crates/config/src/sections/secrets.rs @@ -240,28 +240,6 @@ impl From for EncryptionRaw { } } -/// Description of signing keys. -/// -/// It either holds the key config values directly or references a directory -/// where each file contains a key. -#[derive(Debug, Clone)] -pub enum Keys { - Values(Vec), - Directory(Utf8PathBuf), -} - -impl Keys { - /// Returns a list of key configs. - /// - /// If `keys_dir` was given, the keys are read from file. - async fn key_configs(&self) -> anyhow::Result> { - match self { - Keys::Values(key_configs) => Ok(key_configs.clone()), - Keys::Directory(path) => key_configs_from_path(path).await, - } - } -} - /// Reads all keys from the given directory. async fn key_configs_from_path(path: &Utf8PathBuf) -> anyhow::Result> { let mut result = vec![]; @@ -279,47 +257,6 @@ async fn key_configs_from_path(path: &Utf8PathBuf) -> anyhow::Result>, - - /// Directory of private keys to use for signing and encrypting payloads. - #[schemars(with = "Option")] - #[serde(skip_serializing_if = "Option::is_none")] - keys_dir: Option, -} - -impl TryFrom for Keys { - type Error = anyhow::Error; - - fn try_from(value: KeysRaw) -> Result { - match (value.keys, value.keys_dir) { - (None, None) => bail!("Missing `keys` or `keys_dir`"), - (None, Some(path)) => Ok(Keys::Directory(path)), - (Some(keys), None) => Ok(Keys::Values(keys)), - (Some(_), Some(_)) => bail!("Cannot specify both `keys` and `keys_dir`"), - } - } -} - -impl From for KeysRaw { - fn from(value: Keys) -> Self { - match value { - Keys::Directory(path) => KeysRaw { - keys_dir: Some(path), - keys: None, - }, - Keys::Values(keys) => KeysRaw { - keys_dir: None, - keys: Some(keys), - }, - } - } -} - /// Application secrets #[serde_as] #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] @@ -330,11 +267,14 @@ pub struct SecretsConfig { #[serde(flatten)] encryption: Encryption, - /// List of private keys to use for signing and encrypting payloads - #[schemars(with = "KeysRaw")] - #[serde_as(as = "serde_with::TryFromInto")] - #[serde(flatten)] - keys: Keys, + /// List of private keys to use for signing and encrypting payloads. + #[serde(skip_serializing_if = "Option::is_none")] + keys: Option>, + + /// Directory of private keys to use for signing and encrypting payloads. + #[schemars(with = "Option")] + #[serde(skip_serializing_if = "Option::is_none")] + keys_dir: Option, } impl SecretsConfig { @@ -345,7 +285,7 @@ impl SecretsConfig { /// Returns an error when a key could not be imported #[tracing::instrument(name = "secrets.load", skip_all)] pub async fn key_store(&self) -> anyhow::Result { - let key_configs = self.keys.key_configs().await?; + let key_configs = self.key_configs().await?; let web_keys = try_join_all(key_configs.iter().map(KeyConfig::json_web_key)).await?; Ok(Keystore::new(JsonWebKeySet::new(web_keys))) @@ -380,6 +320,21 @@ impl SecretsConfig { } } } + + /// Returns a combined list of key configs given inline and from files. + /// + /// If `keys_dir` was given, the keys are read from file. + async fn key_configs(&self) -> anyhow::Result> { + let mut key_configs = match &self.keys_dir { + Some(keys_dir) => key_configs_from_path(keys_dir).await?, + None => vec![], + }; + + let inline_key_configs = self.keys.as_deref().unwrap_or_default(); + key_configs.extend(inline_key_configs.iter().cloned()); + + Ok(key_configs) + } } impl ConfigurationSection for SecretsConfig { @@ -461,7 +416,8 @@ impl SecretsConfig { Ok(Self { encryption: Encryption::Value(Standard.sample(&mut rng)), - keys: Keys::Values(vec![rsa_key, ec_p256_key, ec_p384_key, ec_k256_key]), + keys: Some(vec![rsa_key, ec_p256_key, ec_p384_key, ec_k256_key]), + keys_dir: None, }) } @@ -502,7 +458,8 @@ impl SecretsConfig { Self { encryption: Encryption::Value([0xEA; 32]), - keys: Keys::Values(vec![rsa_key, ecdsa_key]), + keys: Some(vec![rsa_key, ecdsa_key]), + keys_dir: None, } } } @@ -619,7 +576,7 @@ mod tests { ] ); - let mut key_config = config.keys.key_configs().await.unwrap(); + let mut key_config = config.key_configs().await.unwrap(); key_config.sort_by_key(|a| { if let Key::File(p) = &a.key { Some(p.clone()) @@ -696,4 +653,57 @@ mod tests { .await .unwrap(); } + + #[tokio::test] + async fn load_config_mixed_key_sources() { + task::spawn_blocking(|| { + Jail::expect_with(|jail| { + jail.create_file( + "config.yaml", + indoc::indoc! {r" + secrets: + encryption_file: encryption + keys_dir: keys + keys: + - kid: lekid0 + key: | + -----BEGIN EC PRIVATE KEY----- + MHcCAQEEIOtZfDuXZr/NC0V3sisR4Chf7RZg6a2dpZesoXMlsPeRoAoGCCqGSM49 + AwEHoUQDQgAECfpqx64lrR85MOhdMxNmIgmz8IfmM5VY9ICX9aoaArnD9FjgkBIl + fGmQWxxXDSWH6SQln9tROVZaduenJqDtDw== + -----END EC PRIVATE KEY----- + "}, + )?; + jail.create_dir("keys")?; + jail.create_file( + "keys/key_from_file", + indoc::indoc! {r" + -----BEGIN EC PRIVATE KEY----- + MHcCAQEEIKlZz/GnH0idVH1PnAF4HQNwRafgBaE2tmyN1wjfdOQqoAoGCCqGSM49 + AwEHoUQDQgAEHrgPeG+Mt8eahih1h4qaPjhl7jT25cdzBkg3dbVks6gBR2Rx4ug9 + h27LAir5RqxByHvua2XsP46rSTChof78uw== + -----END EC PRIVATE KEY----- + "}, + )?; + + let config = Figment::new() + .merge(Yaml::file("config.yaml")) + .extract_inner::("secrets")?; + + Handle::current().block_on(async move { + let key_config = config.key_configs().await.unwrap(); + let key_store = config.key_store().await.unwrap(); + + assert!(key_config[0].kid.is_none()); + assert!(matches!(&key_config[0].key, Key::File(p) if p == "keys/key_from_file")); + assert!(key_store.iter().any(|k| k.kid() == Some("ONUCn80fsiISFWKrVMEiirNVr-QEvi7uQI0QH9q9q4o"))); + assert!(key_store.iter().any(|k| k.kid() == Some("lekid0"))); + }); + + Ok(()) + }); + }) + .await + .unwrap(); + } } From bcf85e8bb59658645b5d1ef52edf11a7e88e234e Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 19 Nov 2025 13:12:20 +0100 Subject: [PATCH 4/4] Fix the building of the docs with mdbook 0.5 --- book.toml | 3 +-- misc/build-docs.sh | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/book.toml b/book.toml index 88f5e8263..fa1f0cfeb 100644 --- a/book.toml +++ b/book.toml @@ -7,9 +7,8 @@ # https://rust-lang.github.io/mdBook/format/config.html [book] title = "Matrix Authentication Service" -authors = ["The Matrix.org Foundation C.I.C."] +authors = ["Element Backend Team"] language = "en" -multilingual = false src = "docs" diff --git a/misc/build-docs.sh b/misc/build-docs.sh index caeab8d85..12818f8af 100644 --- a/misc/build-docs.sh +++ b/misc/build-docs.sh @@ -14,7 +14,7 @@ set -eux # Install the dependencies if we're in the Cloudflare Pages build environment # In this environment, the CF_PAGES environment variable is set to 1 if [ "${CF_PAGES:-""}" = "1" ]; then - MDBOOK_VERSION=0.4.32 + MDBOOK_VERSION=0.5.0 # Install rustup curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- --default-toolchain none -y