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 a7e7c3c

Author: networkException

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 => {

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<PublicJsonWebKeySet> 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<String>")]
-    #[serde(skip_serializing_if = "Option::is_none")]
-    client_secret_file: Option<Utf8PathBuf>,
-
-    /// Alternative to `client_secret_file`: Reads the client secret directly
-    /// from the config.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    client_secret: Option<String>,
-}
-
-impl TryFrom<ClientSecretRaw> for Option<ClientSecret> {
-    type Error = anyhow::Error;
-
-    fn try_from(value: ClientSecretRaw) -> Result<Self, Self::Error> {
-        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<Option<ClientSecret>> for ClientSecretRaw {
-    fn from(value: Option<ClientSecret>) -> 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<Option<String>> {
         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,
         })
     }

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<String>")]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    client_secret_file: Option<Utf8PathBuf>,
+
+    /// Alternative to `client_secret_file`: Reads the client secret directly
+    /// from the config.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    client_secret: Option<String>,
+}
+
+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<String> {
+        Ok(match self {
+            ClientSecret::File(path) => tokio::fs::read_to_string(path).await?,
+            ClientSecret::Value(client_secret) => client_secret.clone(),
+        })
+    }
+}
+
+impl TryFrom<ClientSecretRaw> for Option<ClientSecret> {
+    type Error = anyhow::Error;
+
+    fn try_from(value: ClientSecretRaw) -> Result<Self, Self::Error> {
+        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<Option<ClientSecret>> for ClientSecretRaw {
+    fn from(value: Option<ClientSecret>) -> 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,
+            },
+        }
+    }
+}

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<String>,
+    #[schemars(with = "ClientSecretRaw")]
+    #[serde_as(as = "serde_with::TryFromInto<ClientSecretRaw>")]
+    #[serde(flatten)]
+    pub client_secret: Option<ClientSecret>,
 
     /// 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<Option<String>> {
+        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::<UpstreamOAuth2Config>("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();
+    }
+}

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,