diff --git a/crates/cli/src/sync.rs b/crates/cli/src/sync.rs index 4b8c388c3..bb9048c14 100644 --- a/crates/cli/src/sync.rs +++ b/crates/cli/src/sync.rs @@ -201,25 +201,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 761f1dd62..587952b5a 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2037,10 +2037,6 @@ "description": "The client ID to use when authenticating with the provider", "type": "string" }, - "client_secret": { - "description": "The client secret to use when authenticating with the provider\n\nUsed by the `client_secret_basic`, `client_secret_post`, and `client_secret_jwt` methods", - "type": "string" - }, "token_endpoint_auth_method": { "description": "The method to authenticate the client with the provider", "allOf": [ @@ -2161,6 +2157,14 @@ "$ref": "#/definitions/OnBackchannelLogout" } ] + }, + "client_secret_file": { + "description": "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.", + "type": "string" + }, + "client_secret": { + "description": "Alternative to `client_secret_file`: Reads the client secret directly from the config.", + "type": "string" } } },